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

Failures of some tests for load_confounds on mac OS 14 #4404

Closed
Remi-Gau opened this issue Apr 25, 2024 · 11 comments · Fixed by #4411
Closed

Failures of some tests for load_confounds on mac OS 14 #4404

Remi-Gau opened this issue Apr 25, 2024 · 11 comments · Fixed by #4411

Comments

@Remi-Gau
Copy link
Collaborator

Remi-Gau commented Apr 25, 2024

See: https://github.com/nilearn/nilearn/actions/runs/8831115070

https://github.com/nilearn/nilearn/actions/runs/8831115070/job/24263341338#step:6:16805

FAILED nilearn/interfaces/fmriprep/tests/test_load_confounds.py::test_nilearn_standardize[False-True-zscore] - AssertionError: assert 0.40940820609275674 < 0.2
FAILED nilearn/interfaces/fmriprep/tests/test_load_confounds.py::test_nilearn_standardize[False-True-psc] - AssertionError: assert 0.31965505826115304 < 0.2
FAILED nilearn/interfaces/fmriprep/tests/test_load_confounds.py::test_nilearn_standardize[True-True-zscore] - AssertionError: assert 0.39206694247720847 < 0.2
FAILED nilearn/interfaces/fmriprep/tests/test_load_confounds.py::test_nilearn_standardize[True-True-psc] - AssertionError: assert 0.31224780492654064 < 0.2

OS and dependencies details

Operating System
  macOS
  14.4.1
  23E224
Runner Image
  Image: macos-14-arm64
  Version: 20240422.3
  Included Software: https://github.com/actions/runner-images/blob/macos-14-arm64/20240422.3/images/macos/macos-14-arm64-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20240422.3


certifi==2024.2.2,charset-normalizer==3.3.2,contourpy==1.2.1,coverage==7.5.0,cycler==0.12.1,fonttools==4.51.0,
idna==3.7,iniconfig==2.0.0,joblib==1.4.0,kaleido==0.2.1,kiwisolver==1.4.5,lxml==5.2.1,
matplotlib==3.8.4,nibabel==5.2.1,
nilearn @ file:///Users/runner/work/nilearn/nilearn/.tox/.tmp/package/1/nilearn-0.1.dev1%2Bgd2ddfb9.tar.gz#sha256=373d47decc53f9115b39dde15210c9ab287567247e8980a2f1118e36ecf92915,
numpy==1.26.4,packaging==24.0,pandas==2.2.2,pillow==10.3.0,pip==24.0,plotly==5.21.0,
pluggy==1.5.0,pyparsing==3.1.2,pytest==8.1.1,pytest-cov==5.0.0,python-dateutil==2.9.0.post0,
pytz==2024.1,requests==2.31.0,scikit-learn==1.4.2,scipy==1.13.0,six==1.16.0,tenacity==8.2.3,
threadpoolctl==3.4.0,tzdata==2024.1,urllib3==2.2.1
@Remi-Gau
Copy link
Collaborator Author

Last successful run on main

https://github.com/nilearn/nilearn/actions/runs/8831115070/job/24245630266#step:1:11

Operating System
  macOS
  12.7.4
  21H1123
Runner Image
  Image: macos-12
  Version: 20240418.1
  Included Software: https://github.com/actions/runner-images/blob/macOS-12/20240418.1/images/macos/macos-12-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/macOS-12%2F20240418.1

certifi==2024.2.2,charset-normalizer==3.3.2,contourpy==1.2.1,coverage==7.5.0,cycler==0.12.1,fonttools==4.51.0,
idna==3.7,iniconfig==2.0.0,joblib==1.4.0,kaleido==0.2.1,kiwisolver==1.4.5,lxml==5.2.1,
matplotlib==3.8.4,nibabel==5.2.1,
nilearn @ file:///Users/runner/work/nilearn/nilearn/.tox/.tmp/package/1/nilearn-0.1.dev1%2Bgd2ddfb9.tar.gz#sha256=373d47decc53f9115b39dde15210c9ab287567247e8980a2f1118e36ecf92915,
numpy==1.26.4,packaging==24.0,pandas==2.2.2,pillow==10.3.0,pip==24.0,plotly==5.21.0,
pluggy==1.5.0,pyparsing==3.1.2,pytest==8.1.1,pytest-cov==5.0.0,python-dateutil==2.9.0.post0,
pytz==2024.1,requests==2.31.0,scikit-learn==1.4.2,scipy==1.13.0,six==1.16.0,tenacity==8.2.3,
threadpoolctl==3.4.0,tzdata==2024.1,urllib3==2.2.1

@Remi-Gau Remi-Gau changed the title Failures of some tests for load_confounds on mac OS Failures of some tests for load_confounds on latest mac OS Apr 25, 2024
@Remi-Gau
Copy link
Collaborator Author

seems that this is OS version related

@bthirion
Copy link
Member

We may not be the only ones facing this...

@Remi-Gau
Copy link
Collaborator Author

We may not be the only ones facing this...

Yes there are times in my life when I want to feel special and I seriously hope this is not one those.

@Remi-Gau
Copy link
Collaborator Author

Confirmed here that previous mac OS versions did not fail:

#4405

@Remi-Gau
Copy link
Collaborator Author

Until this gets resolved I am tempted to change CI so to test on both macos 13 and 14 so make it clear on seeing tests results that this an OS issue.

@bthirion
Copy link
Member

Indeed

@Remi-Gau Remi-Gau changed the title Failures of some tests for load_confounds on latest mac OS Failures of some tests for load_confounds on mac OS 14 Apr 26, 2024
@Remi-Gau
Copy link
Collaborator Author

Testing things in a separate repo to iterate more quickly: https://github.com/Remi-Gau/nilearn_tmp

As far as I can tell the tests only fail when using detrend when masking the data with confounds.

So far I have not seen anyone raise an issue about this but I think that my google / github fu is failing me.

@htwangtw
If you have a bit of time do you think you could check this issue.

Ideally I think we'd want to have a very reduced example demonstrating the issue, so we can report it in the correct place.

@htwangtw
Copy link
Member

I feel like I have been putting this off for too long... In this set of tests I was just checking if the denoising gets some value below an arbitrary range, therefore numerical instability can really tip things off... I will have to refresh my memory and think of a better set of tests

@htwangtw
Copy link
Member

htwangtw commented May 7, 2024

I am going to work on this today and tomorrow

@htwangtw
Copy link
Member

htwangtw commented May 7, 2024

With some help from @anibalsolon, we noticed that it was all the test with detrend options failing,and the value was really close to 0. We have seen similar problems with detrend and psc options both enabled. In the test I added trend in the simulated data and now it is passing. Will clean up the PR a bit

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 a pull request may close this issue.

3 participants