Skip to content
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

dunamai RuntimeError and UnboundLocalError: local variable 'nZ' referenced before assignment #30

Open
astewartau opened this issue Jan 15, 2024 · 9 comments

Comments

@astewartau
Copy link

astewartau commented Jan 15, 2024

Thanks for your work on nii2dcm! I'm considering integrating this as part of my QSM processing toolbox, QSMxT.

However, I'm having problems using it in my miniconda setup:

(qsmxt) ~/.../data/qsmtest/qsm: nii2dcm sub-1_ses-20231020_part-phase_T2Starw_romeo-unwrapped_normalized_pdf_rts_twopass_ref.nii dicom-output-directory/ --dicom-type MR
Traceback (most recent call last):
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/bin/nii2dcm", line 5, in <module>
    from nii2dcm.__main__ import cli
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/__main__.py", line 9, in <module>
    from nii2dcm._version import __version__
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/_version.py", line 2, in <module>
    __version__ = Version.from_git().serialize(metadata=False, style=Style.SemVer)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/dunamai/__init__.py", line 1058, in from_git
    _detect_vcs(vcs)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/dunamai/__init__.py", line 355, in _detect_vcs
    raise RuntimeError(
RuntimeError: This does not appear to be a Git project

It is related somehow to the dunamai package. Any idea what could be causing this?

I tried initialising a git repository in the local directory to see if that could be a workaround, but then I have a new problem:

(qsmxt) ~/.../data/qsmtest/qsm: nii2dcm -d MR sub-1_ses-20231020_part-phase_T2Starw_romeo-unwrapped_normalized_pdf_rts_twopass_ref.nii dicom-output-directory/
Traceback (most recent call last):
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/bin/nii2dcm", line 8, in <module>
    sys.exit(cli())
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/__main__.py", line 55, in cli
    run_nii2dcm(
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/run.py", line 38, in run_nii2dcm
    nii2dcm_parameters = nii2dcm.nii.Nifti.get_nii2dcm_parameters(nii)
  File "/home/ashley/neurodesktop-storage/miniconda3/envs/qsmxt/lib/python3.8/site-packages/nii2dcm/nii.py", line 38, in get_nii2dcm_parameters
    nInstances = nZ*nF
UnboundLocalError: local variable 'nZ' referenced before assignment

Looking at your code in nii.py, I can see that this is happening because of the dimensions of my file:

>>> nii.header['dim']
array([  3, 164, 205, 205,   0,   0,   0,   0], dtype=int16)

Which will not be handled correctly by the initialization code:

https://github.com/tomaroberts/nii2dcm/blob/09d20e02f61da2aa5e7a2e57bd420828021f0d92/nii2dcm/nii.py#L29C1-L35C58

My file is a 3D volume with 205 slices, each with dimensions 164x205. It is a quantitative susceptibility map with floating-point values in the range of about -4 to +3, with most values around zero.

@astewartau astewartau changed the title RuntimeError related to dunamai dunamai RuntimeError and UnboundLocalError: local variable 'nZ' referenced before assignment Jan 15, 2024
@astewartau
Copy link
Author

astewartau commented Jan 15, 2024

I've implemented a quick and crude solution for this in my fork to work only for my QSM images:

astewartau@ccd2ba5

I also solved another problem specific to my data - QSM images in NIfTI pipelines are floating-point values centred around zero, though they can have outliers in either direction. DICOMs for QSM images are usually centred around 2048, so a robust conversion is required, which I've tried to do in my updated run.py.

@astewartau
Copy link
Author

I also added a command-line option to enable this. :)

astewartau@b54fd20

If you think this would be useful, I can create a PR or you could let me know if there's a better way?

@tomaroberts
Copy link
Owner

@astewartau – thanks for submitting the Issue! I've got a very busy week, but will look into this soon.

On the miniconda part – I tend to use pip with my virtual environments, so I haven't tested it extensively with other package managers. This is not a suggestion as a solution, but as a sanity check, have you tried installing nii2cm into a fresh venv via pip following the README instructions?

Thanks for the code changes – I'll look at them properly and then get back to you about integration/PR.

@astewartau
Copy link
Author

Hi @tomaroberts, thanks for offering to look into this! I don't get the git issue when I use a virtual environment - it seems to only occur in my Miniconda setup.

Just a little more information that might be helpful - the susceptibility map uses float data, which I've learned we can store in DICOM if we use the FloatPixelData part of the pydicom Dataset object rather than PixelData, which would mean we don't need to force/scale the data to fit uint16 like I've attempted. However, I'm having issues creating a dummy DICOM file with FloatPixelData via pydicom. This might be relevant:

pydicom/pydicom#1077

@tomaroberts
Copy link
Owner

tomaroberts commented Jan 17, 2024

@astewartau

I'm not very familiar with FloatPixelData, but you might want to also look up the ImagePixel module in the DICOMs (e.g. https://github.com/tomaroberts/nii2dcm/blob/main/nii2dcm/modules/image_pixel.py), because these influence contrast. You approach to scaling to uint16 is not a bad idea to pursue.

I see you've forked the repo. My intention was to have classes for different modalities or subtypes. Your QSM work would align with this.

I haven't fleshed it out fully yet, but you can create Classes which inherit the MRI DICOM modules, such as I have done for SVR: https://github.com/tomaroberts/nii2dcm/blob/main/nii2dcm/svr.py

You could make an equivalent, let's call it qsm.py or whatever you fancy, and then use that to override the default DicomMRI tags in your QMS DICOM. e.g. this might be a way of testing out the windowing.

You may also need to edit the command line code so you can supply QSM as a -d / --dicom-type option, e.g. --dicomtype QSM.

@jcohen02
Copy link
Collaborator

Just a quick addition here to link this with #31 - the first issue you describe at the top of your issue, @astewartau, regarding dunamai looks like it can be addressed by the workarounds in #31 until a permanent fix is put in place. However, it looks like initialising the git repo probably also worked around this 🙂.

The second part where you highlight the UnboundLocalError: local variable 'nZ' referenced before assignment error looks to be something separate. Great to see that you were able to implement a solution to this - I'll leave this issue with you and @tomaroberts but just wanted to flag #31.

@tomaroberts
Copy link
Owner

@astewartau – I've released v0.1.5 of nii2dcm, which fixes the pip install issue, if you want to try it out.

If you are still interested in the integration with your QSM toolbox, let me know and I'll re-read this issue more thoroughly, as it's been a little while.

Thanks.

astewartau added a commit to astewartau/nii2dcm that referenced this issue Apr 12, 2024
@astewartau
Copy link
Author

Thanks so much! That seems to be working well.

I may have some time to come back to the scaling issue for floating-point maps such as QSM soon, but for now I've also created a pull request for the undefined variable nZ issue. This issue occurs with valid 3D NIfTI images that store their dimensions slightly differently than assumed (see original post).

#39

Hopefully the fix makes sense! :)

@tomaroberts
Copy link
Owner

Great! Thanks for PR. Will take a look :)

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

No branches or pull requests

3 participants