fix: add missing D_and_Ds_swap() to 13 algorithms for consistent parameter labelling#147
fix: add missing D_and_Ds_swap() to 13 algorithms for consistent parameter labelling#147Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Conversation
…meter labelling Add self.D_and_Ds_swap(results) call before return in ivim_fit() for: - IAR_LU_segmented_3step, IAR_LU_subtracted - PV_MUMC_biexp - OGC_AmsterdamUMC_biexp, OGC_AmsterdamUMC_biexp_segmented, OGC_AmsterdamUMC_Bayesian_biexp - OJ_GU_seg, PvH_KB_NKI_IVIMfit, TF_reference_IVIMfit - TCML_TechnionIIT_SLS, TCML_TechnionIIT_lsq_sls_lm/trf/BOBYQA This corrects mislabelled D and D* when the optimizer swaps them. Previously only 9/22 algorithms had this correction. No regressions: 1127 passed, 167 skipped, 22 xfailed, 6 xpassed.
|
This PR adds the missing Previously, only 9 out of 22 Python algorithms applied this correction, leading to inconsistent parameter labelling. Changes MadeAdded
Each change is a single +1 line addition. No other code was modified. Testing/Verification
|
|
Some discussion and history here: @IvanARashid do I remember this correctly? |
|
I didn't know about this. I think you are right about the edge case when f ≈ 0 (mono-exponential, no perfusion), D fits normally but D* becomes meaninglessly small (e.g. D* → 0.001). This triggers the swap condition (D > Dp AND Dp < 0.05), which incorrectly swaps D and D* and flips f, messing up an otherwise valid fit. I have 2 options here (I think and correct me if I am wrong) - if results['D'] > results['Dp'] and results['Dp'] < 0.05 and results['f'] > 0.01:
# only swap when there's a meaningful perfusion fractionI will need your confirmation here like what to do. |
Add self.D_and_Ds_swap(results) call before return in ivim_fit() for:
This corrects mislabelled D and D* when the optimizer swaps them. Previously only 9/22 algorithms had this correction.
No regressions: 1127 passed, 167 skipped, 22 xfailed, 6 xpassed.
Describe the changes you have made in this PR
A clear and concise description of what you want to happen
Link this PR to an issue [optional]
Fixes #146
Checklist