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

Warning with numpy.longdouble gets thrown on WSL when calling SCT functions #4411

Closed
sandrinebedard opened this issue Mar 20, 2024 · 9 comments
Assignees
Labels
bug category: fixes an error in the code OS: Windows (WSL) priority:LOW upstream Issue caused by software dependencies

Comments

@sandrinebedard
Copy link
Member

Description

I intalled the lastest master's version (git-master-56c7f6cdb0f763f1e287a3255b25ab532e257a00*)

And when running sct_check_dependencies, I get this user warning:

Check if monai is installed........................./mnt/c/Users/sb199/spinalcordtoolbox/python/envs/venv_sct/lib/python3.9/site-packages/numpy/core/getlimits.py:542: UserWarning: Signature b'\x00\xd0\xcc\xcc\xcc\xcc\xcc\xcc\xfb\xbf\x00\x00\x00\x00\x00\x00' for <class 'numpy.longdouble'> does not match any known type: falling back to type probe function.
This warnings indicates broken support for the dtype!
  machar = _get_machar(dtype)

I also get it if I run an sct command:

(base) sabeda@DESKTOP-JQ8A4UV:/mnt/c/Users/sb199/spinalcordtoolbox$ sct_image -h
/mnt/c/Users/sb199/spinalcordtoolbox/python/envs/venv_sct/lib/python3.9/site-packages/numpy/core/getlimits.py:542: UserWarning: Signature b'\x00\xd0\xcc\xcc\xcc\xcc\xcc\xcc\xfb\xbf\x00\x00\x00\x00\x00\x00' for <class 'numpy.longdouble'> does not match any known type: falling back to type probe function.
This warnings indicates broken support for the dtype!
  machar = _get_machar(dtype)

--
Spinal Cord Toolbox (git-master-56c7f6cdb0f763f1e287a3255b25ab532e257a00*)

sct_image -h
--
@joshuacwnewton
Copy link
Member

Could you share the full output of sct_check_dependencies?

I'm not getting this on my end, nor have I seen it in our test suite, so I'm curious what your setup is like. :)

@joshuacwnewton
Copy link
Member

Ah wait, I see now from the other issue that you're on WSL. And, checking the WSL CI logs, I see this too: https://github.com/spinalcordtoolbox/spinalcordtoolbox/actions/runs/8357890181/job/22877996583#step:8:42

 Check if matplotlib is installed....................[OK] (3.8.3)
/root/sct_6.3.dev0/python/envs/venv_sct/lib/python3.9/site-packages/numpy/core/getlimits.py:542: UserWarning: Signature b'\x00\xd0\xcc\xcc\xcc\xcc\xcc\xcc\xfb\xbf\x00\x00\x00\x00\x00\x00' for <class 'numpy.longdouble'> does not match any known type: falling back to type probe function.
This warnings indicates broken support for the dtype!
  machar = _get_machar(dtype)
Check if monai is installed.........................[OK] (1.3.0)

@joshuacwnewton joshuacwnewton changed the title Warning with numpy.longdouble Warning with numpy.longdouble gets thrown on WSL when calling SCT functions Mar 20, 2024
@joshuacwnewton joshuacwnewton self-assigned this Mar 20, 2024
@joshuacwnewton joshuacwnewton added bug category: fixes an error in the code OS: Windows (WSL) labels Mar 20, 2024
@joshuacwnewton
Copy link
Member

Some relevant issues:

It seems like this is an issue with WSL itself, rather than numpy. The warning itself is a general warning that has been present in numpy for at least 8 years: numpy/numpy@a611932.

Are you using WSL1 or WSL2? Because it seems like this issue may be fixed in WSL2: numpy/numpy#22187 (comment)

@sandrinebedard
Copy link
Member Author

I am using WSL1, but I never had that warning before in SCT

@joshuacwnewton
Copy link
Member

I never had that warning before in SCT

I think this is due to #4332, which caused us to upgrade numpy versions. According to h5py/h5py#2357, downgrading to 1.24 removes the warning, and we were on 1.23.5 until a few days ago.

@joshuacwnewton
Copy link
Member

Hrm. Given that we explicitly recommend using WSL1 with SCT, there doesn't seem to be an easy way around this.

I wonder where np.longdouble is actually used. We don't use it inside of SCT, but I'm not so sure about our dependencies. If they use np.longdouble in a meaningful way that we trigger during processing, does that mean our results have been affected by WSL's buggy implementation?

On the other hand, if this warning doesn't truly affect SCT, then perhaps we could filter it instead.

@joshuacwnewton
Copy link
Member

joshuacwnewton commented Mar 22, 2024

I wonder where np.longdouble is actually used.

I set a breakpoint at the LOC where this warning is triggered, then ran sct_check_dependencies.

It looks like the LOC is only traversed when nibabel is imported:

  • import monai -> monai.apps -> monai.datasets -> monai.data -> monai.Dataloader -> monai.data.utils -> import nibabel
  • import nibabel -> nibabel.analyze -> nibabel.arrayproxy -> nibabel.volumeutils -> nibabel.casting -> casting.ok_floats()
  • nibabel.casting.ok_floats() -> best_float() -> type_info(np.longdouble) -> np.finfo(np.longdouble) -> warning is thrown

The source code for best_float() actually seems like it is trying to determine when np.longdouble shouldn't be used. So, I think that answers the question of whether our code is actually negatively affected by the buggy WSL behavior -- nibabel will presumably fall back to float64.

As far as the warning goes -- it seems like this should be reported upstream? It's possible that nibabel isn't aware that the warning that gets thrown, given the narrow circumstances (numpy>=1.25, WSL1). And, they themselves may want to silence this warning, given that they explicitly opt-out of using np.longdouble for Windows.

EDIT: Done:

@joshuacwnewton joshuacwnewton added priority:LOW upstream Issue caused by software dependencies labels Mar 22, 2024
@joshuacwnewton
Copy link
Member

My fix was merged upstream! :)

It will take some time for a new nibabel release to be created -- it seems like my PR was the first to be added to nibabel's 6.0 milestone.

ETA April. Might aim for late March or early May, as April will be busy for me.

Perhaps we could add the same warning filter in the meantime? Otherwise, I think this issue is resolved now.

@joshuacwnewton
Copy link
Member

I think, because this has been fixed upstream it might be best to just close this as completed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code OS: Windows (WSL) priority:LOW upstream Issue caused by software dependencies
Projects
None yet
Development

No branches or pull requests

2 participants