ParameterIndexDoubleEnded.alpha off-by-one when fix_gamma=True (overlaps with ta indices)

[bdestombe]

The alpha property of ParameterIndexDoubleEnded (src/dtscalibration/dts_accessor_utils.py:115-122) returns one too many indices in the fix_gamma=True, fix_alpha=False configuration. The extra index overlaps with the start of the ta (transient-attenuation) range, so p_val[ip.alpha] reads one trailing ta slot as if it were an alpha, and p_val[ip.ta] shares its first slot with alpha's last.

The bug

@property
def alpha(self):
    if self.fix_alpha:
        return []
    elif self.fix_gamma:
        return list(range(2 * self.nt, 1 + 2 * self.nt + self.nx))   # <-- nx+1 indices
    elif not self.fix_gamma:
        return list(range(1 + 2 * self.nt, 1 + 2 * self.nt + self.nx))  # nx indices, correct

When fix_gamma=True, the gamma slot is removed from the parameter vector and every following block should shift down by one. df, db, and ta are written correctly with this in mind:

df  -> range(self.nt)                                  # nt indices, [0, nt)
db  -> range(self.nt, 2 * self.nt)                     # nt indices, [nt, 2*nt)
ta  -> np.arange(2 * self.nt + self.nx, self.npar)     # 2*nt*nta indices

Only the alpha branch keeps the leading 1 + from the no-fix layout, which yields nx + 1 indices instead of nx.

Reproducer

from dtscalibration.dts_accessor_utils import ParameterIndexDoubleEnded

nt, nx, nta = 5, 8, 2
ip = ParameterIndexDoubleEnded(nt, nx, nta, fix_gamma=True, fix_alpha=False)

assert ip.npar == 2*nt + nx + 2*nt*nta            # 38, correct
assert len(ip.alpha) == nx                         # FAILS: actual is nx+1=9
# alpha = [10, 11, 12, 13, 14, 15, 16, 17, 18]
# ta    = [18, 19, ..., 37]
# index 18 belongs to both

The other three (fix_gamma, fix_alpha) combinations tile [0, npar) correctly; only this one fails.

Impact

Whenever a user calls calibrate_double_ended(..., fix_gamma=value) without also passing fix_alpha, the calibration solver populates p_val[ip.alpha] and p_val[ip.ta] from overlapping slots. The recovered alpha array picks up the value the solver intended for talpha_fw[t=0, trans_att=0], and talpha_fw_full for that splice loses its first entry. There is no error, just silently wrong calibration outputs in this configuration.

Fix

elif self.fix_gamma:
    return list(range(2 * self.nt, 2 * self.nt + self.nx))

i.e. drop the leading 1 + from the upper bound. Suggest also asserting len(self.alpha) + len(self.df) + len(self.db) + len(self.gamma) + len(self.ta.flatten()) == self.npar somewhere in __init__ (or equivalent invariant property) to catch the next analogous mistake.

Existing test that exposes this

tests/test_b_suite.py::test_b5_parameter_index_double_ended_tiles_npar[False-True] (added in #234, currently xfailed against this issue once filed).

Related

Filed during the code-review follow-up that produced #229-#233. Not covered by any of those.

[BSchilperoort]

Would be good to write a clear explanation of the consequences of this error in the changelog once it's fixed. This will affect calibration results right?

[bdestombe]

This would have raised an error in case of fix_gamm because alpha would be of size nx+1 instead of nx. Luckily no silent errors calibration results.

The proposed solution is wrong though.