feature: Add Weighted Least Squares (WLS) IVIM fitting algorithm — DT_IIITN (Feature #110)#148
Open
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Open
feature: Add Weighted Least Squares (WLS) IVIM fitting algorithm — DT_IIITN (Feature #110)#148Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Conversation
Implement Weighted Least Squares (WLS) segmented IVIM fitting following Veraart et al. (2013) NeuroImage 81:335-346. Algorithm: - Step 1: Fit D from high b-values via WLS on log-signal (w=S^2) - Step 2: Fit D* from residuals at low b-values via WLS New files: - src/original/DT_IIITN/wls_ivim_fitting.py (raw algorithm, numpy only) - src/standardized/DT_IIITN_WLS.py (OsipiBase standardized wrapper) Modified files: - tests/IVIMmodels/unit_tests/algorithms.json (register for automated tests) Follows repository contribution structure: - Fit code in src/original/Initials_Institution/ - Standardized wrapper in src/standardized/ - Registered in algorithms.json (no custom tests needed) Test results: 1184 passed, 167 skipped, 22 xfailed, 6 xpassed. 27 test_volume errors are pre-existing (FileNotFoundError on Windows).
| required_bvalues = 4 | ||
| required_thresholds = [0, 0] | ||
| required_bounds = False | ||
| required_bounds_optional = True |
Collaborator
There was a problem hiding this comment.
@IvanARashid , I'm a bit confused what the difference is between
requiered bounds
requiered bounds optional
supported bounds
Does it make sense for the second to be true and the others false?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello @oliverchampion. The man who was assigned this issue didnt update his PR and had issues in it so I worked according to "how to commit" given in readme. I have also taken 1 decision explained in this PR (
statsmodels.robust.robust_linear_model.RLMvs numpy WSL) . If you suggest that we should use the former than latter - please let me know and I will update this PR.What this PR does
Closes #110. Implements a Weighted Least Squares (WLS) segmented IVIM fitting algorithm in accordance with the repository's contribution structure, following the approach recommended in Veraart et al. (2013).
This PR addresses the rejected PR #136, which implemented the same feature but placed code in the wrong location. This PR follows the correct structure:
src/original/Initials_Institution/src/standardized/algorithms.json(no custom test file needed)Algorithm
Segmented two-step WLS approach:
Step 1 — Estimate D (high b-values, b ≥ 200 s/mm²):
At high b-values the perfusion component decays to ~0, leaving:
Weighted linear regression on log-signal with weights w = S(b)² (Veraart correction for heteroscedasticity introduced by the log-transform). Intercept yields f.
Step 2 — Estimate D* (low b-values):
Subtract the diffusion component to isolate the perfusion signal:
Weighted linear regression with weights w = residual² gives D*.
Implementation: Pure
numpy(no extra dependencies). Uses the closed-form weighted normal equations:Files Changed
src/original/DT_IIITN/__init__.pysrc/original/DT_IIITN/wls_ivim_fitting.pysrc/standardized/DT_IIITN_WLS.pytests/IVIMmodels/unit_tests/algorithms.jsonDT_IIITN_WLSAPI Usage
Design Decision: numpy WLS vs statsmodels RLM
The issue description suggests using
statsmodels.robust.robust_linear_model.RLM. After evaluating both approaches:statsmodelsThe numpy WLS approach implements the same weighting strategy recommended by Veraart et al. (w = S²) but using the closed-form normal equations — which is exactly what the paper validates. The
statsmodels.RLMiterative approach adds robustness to outliers but at a ~15× runtime cost that is impractical for voxel-wise fitting on large datasets.If you (anyone who will see my pr) prefers the full RLM approach, it can be easily switched in
wls_ivim_fitting.pyby replacing the_weighted_linregcall withsm.RLM(...).fit().Reference
Veraart, J. et al. (2013). "Weighted linear least squares estimation of diffusion MRI parameters: strengths, limitations, and pitfalls." NeuroImage, 81, 335–346.
DOI: 10.1016/j.neuroimage.2013.05.028
Testing
Testing is handled automatically by the existing framework via
algorithms.jsonregistration.DT_IIITN_WLS)test_ivim_fit_savedtest_volume/test_parallelerrorsmain(WindowsFileNotFoundErrorin subprocess, unrelated to this PR — confirmed by running same tests onmainbefore this branch)Sample fit result on synthetic signal (
f=0.10, D=0.0010, D*=0.010):Checklist
src/original/Initials_Institution/+src/standardized/contribution structurealgorithms.jsonnumpyonly —statsmodelsevaluated but removed for performance; see Design Decision above)