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

Test out newer versions of PyTorch (torch>=2.0.0) for compatibility with downstream projects (SCT, ADS) #1304

Merged

Conversation

joshuacwnewton
Copy link
Member

@joshuacwnewton joshuacwnewton commented Sep 27, 2023

Because ivadomed's inference functionality is incorporated downstream in many of NeuroPoly's projects (e.g. SCT, ADS), it must "play nicely" with other packages installed into those downstream projects.

One issue with this is that ivadomed currently pins a (relatively old) version of PyTorch, while many other deep learning packages use a newer version of PyTorch. Because of ivadomed's restriction, it often can't be installed into the same environments as other deep learning packages.

So, this PR tests out newer versions of torch (>=2.0.0), for the purposes of compatibility with libraries such as nnUnet, MONAI, etc.

@joshuacwnewton joshuacwnewton changed the title Test out torch==2.0.0 (base branch: kk/chore_imagio) Test out torch==2.0.0 (base branch: kk/chore_imageio) Sep 27, 2023
requirements.txt Outdated Show resolved Hide resolved
@joshuacwnewton joshuacwnewton force-pushed the jn/test-torch-compatibility_base-kk-chore-imageio branch from 7d839e4 to 4ce4c05 Compare December 4, 2023 19:40
@joshuacwnewton joshuacwnewton changed the title Test out torch==2.0.0 (base branch: kk/chore_imageio) Test out newer versions of PyTorch (torch>=2.0.0) for compatibility with downstream projects (SCT, ADS) Dec 4, 2023
@joshuacwnewton
Copy link
Member Author

Now that #1297 has been merged, I've updated this PR to remove the now-redundant commits from the kk/chore-imageio branch.

I'm starting out by testing only torch==2.0.0, but once the CI passes, I'll also try unpinning torch entirely to see if ivadomed can support the newest versions.

@coveralls
Copy link

coveralls commented Dec 4, 2023

Pull Request Test Coverage Report for Build 7104163036

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 74.269%

Totals Coverage Status
Change from base Build 7091651550: -0.004%
Covered Lines: 4676
Relevant Lines: 6296

💛 - Coveralls

@joshuacwnewton joshuacwnewton marked this pull request as ready for review December 4, 2023 22:20
Copy link
Contributor

@kanishk16 kanishk16 left a comment

Choose a reason for hiding this comment

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

Although I did a sanity check on my end using a GPU, would recommend running the test suite once on the lab's GPU. Just going to pre-approve 🚀

requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
Also, add explanatory comment about why these specific versions are referenced.
@mathieuboudreau
Copy link
Contributor

Testing on ADS now

@mathieuboudreau
Copy link
Contributor

It's going to take me a bit longer than expected to test, because since imageio was updated from version 2->3 in #1297, I'm going to have to update ADS to make it compatible with this new imageio as well (basically converting all our as_gray alls) before I can attempt to run the tests for this branch

@mathieuboudreau
Copy link
Contributor

@joshuacwnewton everything looks like it's working on the ADS side - I tested it out in this PR by pointing the installation to the latest commit: https://github.com/axondeepseg/axondeepseg/pull/739/files#diff-9efd195f4e9bfb79ccd456a1d8370fafcc4bcb0b00ea3799222667d2ae818533R29

Everything looked good using our napari GUI as well.

So it seems it's a green light from the ADS side! Thanks!

@joshuacwnewton
Copy link
Member Author

joshuacwnewton commented Dec 11, 2023

Thank you so much for your testing (with imageio as well!) ♥️

@joshuacwnewton joshuacwnewton merged commit c52689d into master Dec 11, 2023
12 checks passed
@joshuacwnewton joshuacwnewton deleted the jn/test-torch-compatibility_base-kk-chore-imageio branch December 11, 2023 14:38
@joshuacwnewton joshuacwnewton added this to the new release milestone Dec 11, 2023
@joshuacwnewton joshuacwnewton added installation category: installation-related stuff maintenance labels Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation category: installation-related stuff maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants