Skip to content

Normalize flip_angles in DCEAcquisitionParams.__post_init__ - #167

Merged
MohamedNasser8 merged 3 commits into
OSIPI:mainfrom
ParikshitGupta2605:fix-vfa-numpy-flip-angles
Aug 4, 2026
Merged

Normalize flip_angles in DCEAcquisitionParams.__post_init__#167
MohamedNasser8 merged 3 commits into
OSIPI:mainfrom
ParikshitGupta2605:fix-vfa-numpy-flip-angles

Conversation

@ParikshitGupta2605

Copy link
Copy Markdown
Contributor

Normalization now happens in DCEAcquisitionParams.post_init, the VFA-specific empty check is restored (load-bearing, not dead code since fixed-T1/t1_assumed acquisitions legitimately construct with empty flip_angles), redundant casts in runner.py/vfa.py are cleaned up, and tests are broadened to cover construction-time validation directly (None, scalar, non-numeric, NaN/inf, 2D array, empty-with-t1_assumed). Ready for another look.

@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

Thanks for the earlier feedback, @ltorres6 and @MohamedNasser8 — I've reopened this with the updated approach.

What changed:

Moved the fix from vfa.py into DCEAcquisitionParams.post_init (osipy/common/types.py), so flip_angles is normalized into a validated list[float] once, at construction — accepting lists, tuples, and NumPy arrays, and raising a clear ValueError for None, non-numeric, non-1D, or non-finite input.
Empty flip_angles is intentionally still allowed at the dataclass level — turns out fixed-T1 acquisitions using t1_assumed legitimately construct DCEAcquisitionParams with flip_angles=[] (confirmed via dce_pipeline.py's branch on t1_assumed is not None), so enforcing non-empty at the dataclass level would've broken that workflow.
Restored the empty check inside compute_t1_vfa itself, since that's the actual place where non-empty flip_angles are required, it's load-bearing now, not dead code.
Cleaned up the now-redundant list/np.asarray casts in runner.py and vfa.py that were doing defensive re-casting the dataclass now handles.
Broadened the tests to cover construction-time validation directly (None, scalar, non-numeric, NaN/inf, 2D array, empty-with-t1_assumed), plus a regression test confirming compute_t1_vfa still rejects empty flip_angles.

@MohamedNasser8 MohamedNasser8 self-assigned this Aug 4, 2026

@MohamedNasser8 MohamedNasser8 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ParikshitGupta2605! Left a couple of comments to fix

Comment thread osipy/dce/t1_mapping/vfa.py Outdated
Comment thread osipy/dce/t1_mapping/vfa.py
@ParikshitGupta2605
ParikshitGupta2605 force-pushed the fix-vfa-numpy-flip-angles branch from 27bc348 to 5d85896 Compare August 4, 2026 21:34
@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

@MohamedNasser8 - addressed both: Pushed both fixes, threads resolved. Tests are passing locally. Let me know if you need anything else!

@MohamedNasser8
MohamedNasser8 merged commit a7df2cf into OSIPI:main Aug 4, 2026
11 checks passed
@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

Hi @MohamedNasser8, I'd like to get more involved with OSIPI and contribute to the project. Would it be possible to send me an invite to the OSIPI Slack workspace so I can introduce myself in the project channel, as mentioned in the contributing guidelines? Thank you!

@MohamedNasser8

Copy link
Copy Markdown
Collaborator

Hey @ParikshitGupta2605 you can join by sending email to ismrmosipi@gmail.com, they will invite you to slack workspace.
Sorry for the delay, didn't notice it.

@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

Thanks for letting me know! No worries about the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants