feat: Add body-part aware initial guesses for IVIM fitting (Feature #87)#149
feat: Add body-part aware initial guesses for IVIM fitting (Feature #87)#149Devguru-codes wants to merge 3 commits intoOSIPI:mainfrom
Conversation
…SIPI#87) Add literature-sourced default initial guesses, bounds, and thresholds for 8 anatomical regions: brain, liver, kidney, prostate, pancreas, head_and_neck, breast, placenta. New files: - src/wrappers/ivim_body_part_defaults.py: lookup table module with get_body_part_defaults() and get_available_body_parts() - tests/IVIMmodels/unit_tests/test_body_part_defaults.py: 22 unit tests Modified files: - src/wrappers/OsipiBase.py: add body_part parameter to __init__(), support initial_guess as string (e.g. initial_guess='liver') API usage: OsipiBase(algorithm='X', bvalues=b, body_part='liver') OsipiBase(algorithm='X', bvalues=b, initial_guess='brain') 100% backward compatible: body_part=None uses original generic defaults. User-provided bounds/initial_guess always take priority. No regressions: 1127 passed, 167 skipped, 22 xfailed, 6 xpassed.
oliverchampion
left a comment
There was a problem hiding this comment.
Hey DevGuru, thanks for implementing this! Some general comments. As some of these implementation also requiere expert input and consensus (bounds, initial guesses) I have asked the community to comment too.
|
I will make all updates at once when the expert's review will come. |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
I have updated the file. Updates are -
|
|
Hello @oliverchampion sir ,the CI tests failed because they have older initial guess values in the tests. This caused the tests to fail. I am going to update the test file that is test_body_part_defaults.py. Now, coming to values are - Liver: Kidney: Bounds Updated (to match the Broad Physical Bounds logic): Normalization Test Updated (Removed Organs): I will wait to update it. I want your thoughts on these values. Thank you. |
|
@Devguru-codes Thanks, this is a good improvement. I do see failing tests so those should be fixed too. |
Ok @etpeterson sir, @DKuppens sir, @IvanARashid sir, @oliverchampion sir, I just need confirmation on the values i am going to use. I have mentioned the values above. Thank you |
Looks good! |
Thank you @IvanARashid sir for confirming the values. I have now updated those values. Thank you. |
|
I have update the values and when i checked those values with the updated bounds, the test cases passed. The tests failed cause they still have the old values present in them. Please review it @oliverchampion sir. Thank you. |
|
@oliverchampion sir, I researched on it and used AI to figure why CI unit test cases were failing only for macos-
These changes are suggested by AI in unit_test.yml which I think I will need your permission before I update it in this PR. Thank you. |
Hello @etpeterson - I saw no updates were made on this issue so I worked on it. If there is need of improvement, comment it and i will update this pr.
Description
Closes #87. Adds organ-specific initial guesses, bounds, and thresholds for IVIM fitting, sourced from peer-reviewed literature. This enables faster convergence, reduced local minima, and more physiologically plausible results when scanning specific body parts.
Problem
The current generic defaults (
f=0.1, D=0.001, Dp=0.01) are reasonable for the brain but significantly off for other organs:Solution: Literature-Sourced Lookup Table
Each body part also includes organ-specific bounds (tighter than the generic ones) to constrain the optimizer to physiologically plausible regions.
Justification for chosen values:
Changes Made
src/wrappers/ivim_body_part_defaults.pyget_body_part_defaults(),get_available_body_parts()src/wrappers/OsipiBase.pybody_partparam to__init__(), supportinitial_guessas stringtests/.../test_body_part_defaults.pyAPI Usage
Testing
body_part="liver"produced near-perfect results:Known Issue (Pre-existing, Not Introduced by This PR)
When using
body_part=together withalgorithm="IAR_LU_biexp", theIAR_LU_biexpalgorithm's internalset_bounds()crashes withKeyError: 0because it expects bounds as list-of-lists (bounds[0],bounds[1]) but receives dict-format bounds. This is the same bug onmainthat is already documented in Issue #86 and fixed in PR #142 — it reproduces identically withbody_part=None(generic defaults). Once PR #142 is merged, this will work seamlessly.Backward Compatibility
body_part=None(default) → identical to current behaviorinitial_guess=dict→ identical to current behaviorbounds/initial_guessalways override body-part defaultsChecklist
tests/IVIMmodels/unit_tests/test_body_part_defaults.pynumpy,scipyalready required)