Skip to content

FIX: Raise informative error if no t1w or t2w found #347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

scott-huberty
Copy link
Contributor

@scott-huberty scott-huberty commented Feb 21, 2024

Hello!

I am doing some debugging of a script that runs nibabies and noticed I was getting the error shown below. This PR aims to add a more informative RunTimeError for users in the future.

Traceback (most recent call last):
  File "/opt/conda/envs/nibabies/bin/nibabies", line 8, in <module>
    sys.exit(main())
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/cli/run.py", line 59, in main
    retval = build_workflow(config_file)
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/cli/workflow.py", line 76, in build_workflow
    retval["workflow"] = init_nibabies_wf(config.execution.unique_labels)
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/workflows/base.py", line 130, in init_nibabies_wf
    single_subject_wf = init_single_subject_wf(
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/workflows/base.py", line 371, in init_single_subject_wf
    else init_infant_single_anat_wf(**wf_args)
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/workflows/anatomical/base.py", line 661, in init_infant_single_anat_wf
    anat_derivatives_wf = init_anat_derivatives_wf(
  File "/opt/conda/envs/nibabies/lib/python3.10/site-packages/nibabies/workflows/anatomical/outputs.py", line 514, in init_anat_derivatives_wf
    (inputnode, raw_sources, [(source_files, 'in_files')]),
UnboundLocalError: local variable 'raw_sources' referenced before assignment

As far as I can tell Nibabies is looking for a T1w or T2w image, and in my case none were found.

This means that the lines in the if clauses (if num_t1w, .... if num_t2w) in init_anat_derivatives_wf were not being hit, and thus raw_sources was never assigned, ultimately causing an UnboundLocalError downstream when trying to pass raw_sources into another function.

So in this PR I explicitly raise an error if no T1w / T2w images are found, which will hopefully be more informative than the aforementioned UnboundLocalError.

Hopefully this can be helpful to someone else down the line, and feel free to let me know if I'm missing anything.

This is to avoid a less informative unbound local error that will be thrown in workflows.anatomical.outputs.init_anat_derivatives_wf on line 514 because the prior if clauses were never hit and so raw_sources never gets defined
@@ -612,6 +612,9 @@ def init_infant_single_anat_wf(
"This workflow uses only T1w or T2w inputs, but both contrasts are available."
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI - I decided to place the check here, because that is where you seem to be checking for the T[1,2]w images already.

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (3a25507) 33.07% compared to head (ca99456) 33.06%.

❗ Current head ca99456 differs from pull request most recent head e33c722. Consider uploading reports for the commit e33c722 to get more accurate results

Files Patch % Lines
nibabies/workflows/anatomical/base.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
- Coverage   33.07%   33.06%   -0.02%     
==========================================
  Files          55       55              
  Lines        5333     5335       +2     
==========================================
  Hits         1764     1764              
- Misses       3569     3571       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for the contribution!

If you would like, please add yourself to the contributors.json - it should be in the same style as the developers.json

@scott-huberty
Copy link
Contributor Author

LGTM - thanks for the contribution!

If you would like, please add yourself to the contributors.json - it should be in the same style as the developers.json

No problem, done!

@mgxd mgxd merged commit 1829514 into nipreps:master Feb 21, 2024
@scott-huberty scott-huberty deleted the fix_better_error branch February 21, 2024 22:34
mgxd added a commit that referenced this pull request Aug 30, 2024
24.0.0 (August 29, 2024)

This major release includes a substantial refactoring of the pipeline.

One key addition is the addition of the `--level` flag, which can take the arguments minimal, resampling or full. The default is full, which should produce nearly the same results as previous versions. minimal will produce only the minimum necessary to deterministically generate the remaining derivatives. resampling will produce some additional derivatives, intended to simplify resampling with other tools.

The `--derivatives` flag was altered to take arguments in the form `name=/path/to/dir`.
For each directory provided, if a derivative is found - it will be used instead of computing it from scratch. If a derivative is not found, NiBabies will compute it and proceed as usual.

Taken together, these features can allow a dataset provider to run a minimal NiBabies run, targeting many output spaces, while a user can then run a `--derivatives` run to generate additional derivatives in only the output spaces they need. Another use case is to provide an precomputed derivative to override the default NiBabies behavior, enabling easier workarounds for bugs or experimentation with alternatives.

Another new feature is a dynamic anatomical reference, which is set based on surface reconstruction method or through the `--reference-anatomical` flag. Previously, T1w was the default output space. Now, the reference anatomical is determined based on the surface reconstruction method.

Additionally, minor adjustments have been made to MCRIBS surface reconstruction to address failure rates. This is still an on-going investigation, but preliminary results look promising.

This release resolves a number of issues with fieldmaps inducing distortions during correction. Phase difference and direct fieldmaps are now masked correctly, preventing the overestimation of distortions outside the brain. Additionally, we now implement Jacobian weighting during unwarping, which corrects for compression and expansion effects on signal intensity. To disable Jacobian weighting, add `fmap-jacobian` to the `--ignore` argument.

Finally, a new resampling method has been added, to better account for susceptibility distortion and motion in a single shot resampling to a volumetric target space. We anticipate extending this to surface targets in the future.

  * FIX: nest pathlib import in fix_multi_source_name (#365)
  * FIX: Avoid retrieving multiple templates from latest TF (#353)
  * FIX: Raise informative error if no t1w or t2w found (#347)
  * FIX: Easier pyenv usage (#342)
  * FIX: Catch nonexistent derivatives, clean up subworkflow logic (#336)
  * FIX: Use fsLR reg sphere for MCRIBS morphometrics resampling (#334)
  * FIX: T2star map MNI scaling (#320)

  * ENH: Alter outputs when MCRIBS reconstruction is used (#329)
  * ENH: Use nireports for Report generation + add reportlet per reconstruction (#328)
  * ENH: better repr for Derivatives class (#351)

  * RF: Move to fit/apply workflow (#360)
  * RF: Replace `resource_filename` with `load_data` (#345)

  * MAINT: Bump urllib3 from 2.0.3 to 2.0.7 (#319)
  * MAINT: Raise minimum to 3.10, bump actions (#337)
  * MAINT: Bump pillow from 9.5.0 to 10.0.1 (#317)
  * MAINT: Update to latest migas API (#326)

  * DOC: Use correct argument flag (#338)
  * DOC: Move to new theme, add outputs description (#383)
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