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

SCT v5.8 ships with ivadomed==2.9.7 which contains a fatal bug for sct_deepseg #4148

Closed
joshuacwnewton opened this issue Jul 7, 2023 · 1 comment
Labels
bug category: fixes an error in the code sct_deepseg context: Global entry point for all deep learning segmentation methods upstream Issue caused by software dependencies user requested Raised by user on the SCT forum/email/GitHub. Be sure to notify them when fixed in a release.

Comments

@joshuacwnewton
Copy link
Member

joshuacwnewton commented Jul 7, 2023

A user on the forum reported the following issue: https://forum.spinalcordmri.org/t/error-in-seg-tumor-t2-task/1099

The bug is within ivadomed, and causes sct_deepseg to crash. (Note: This issue only affects 3D deepseg models, and not slicewise (2D) deepseg models. Off the top of my head, I don't know what % of our models are 3D and what % are 2D.)

The error that the user encountered is one that has already been reported upstream and fixed: ivadomed/ivadomed#1213. Thus, this is easily fixed by upgrading ivadomed, preferably with the --no-deps option to preserve the rest of the SCT v5.8 environment.

We could backport this fix by bumping the version of ivadomed in 5.8's requirements-freeze.txt. The tricky thing here is ivadomed's dependencies: Is it possible to bump only ivadomed, and leave the other dependencies fixed? Or does ivadomed have strict dependency pinning that makes this impossible? (Assuming there are even meaningful dependency changes between ivadomed v2.9.7 and ivadomed v2.9.8.)

@joshuacwnewton joshuacwnewton added bug category: fixes an error in the code upstream Issue caused by software dependencies sct_deepseg context: Global entry point for all deep learning segmentation methods user requested Raised by user on the SCT forum/email/GitHub. Be sure to notify them when fixed in a release. labels Jul 7, 2023
@joshuacwnewton joshuacwnewton changed the title SCT v5.8 ships with a version of ivadomed containing a fatal bug for sct_deepseg SCT v5.8 ships with ivadomed==2.9.7 which contains a fatal bug for sct_deepseg Jul 7, 2023
@mathieuboudreau
Copy link
Contributor

The tricky thing here is ivadomed's dependencies: Is it possible to bump only ivadomed, and leave the other dependencies fixed? Or does ivadomed have strict dependency pinning that makes this impossible?

In our experience over in AxonDeepSeg, I think we've experienced first of these two things, but I believe because we're installing most of our packages with conda (which strictly enforces requirements) and then ivadomed with pip (which doesn't strictly enforce the versions), using the same environment package. See this snippet in a PR I opened on IVADOMED recently ivadomed/ivadomed#1292 (comment).

SCT does it's installation quite differently than us, so maybe IVADOMED will try to enforce it's requirements more strictly for you on you, unlike us.

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 sct_deepseg context: Global entry point for all deep learning segmentation methods upstream Issue caused by software dependencies user requested Raised by user on the SCT forum/email/GitHub. Be sure to notify them when fixed in a release.
Projects
None yet
Development

No branches or pull requests

3 participants