Showcase the changes present in neuropoly-fork-patches

[joshuacwnewton]

Caution

This PR should not be merged. The purpose of this PR is simply to showcase the changes currently on the neuropoly-fork-patches branch.

instructions

• This branch should be continuously updated against master (to ensure that the changes in this branch are always compatible with the most recent version of nnunetv2)

  # prerequisite: sync the nnUNet-neuropoly `master` with the upstream `master` on GitHub
  git checkout master
  git pull master
  git checkout neuropoly-fork-patches
  git rebase master
  # during rebase: fix any conflicts
  git push --force

• Whenever there is a new upstream nnunetv2 release, we should create a new branch for a parallel release on our own nnunetv2-neuropoly project, then cherry-pick the changes from this neuropoly-fork-changes branch onto the release branch.

[joshuacwnewton]

@plbenveniste I think these are the next steps to take:

• Restore the change name="nnunetv2-neuropoly"
• Create a new "developer" release on "Test PyPI" for this branch (containing your logits fixes)
  • Since my last "developer" release was called "2.6.2.dev1", the next release can be called "2.6.2.dev2"
  • The commands I used were just:

    python3 -m build
    python3 -m twine upload --repository testpypi dist/*

• Update Switch to neuropoly fork of nnunetv2 to support multi-fold "custom trainer" version of lesion_ms spinalcordtoolbox#5027 to use this release.
  • You can copy all of the changes from [WIP] Prototype fork of nnunetv2 with torch<2.3 fixes. spinalcordtoolbox#5003.
  • Some of these changes are temporary "hacks" because of totalspineseg.

Once we confirm that the Test PyPI release works with your PR, and we are happy with the changes, then we can make some more permanent changes:

• Update the release of lesion_ms to include the "trainer class" file in the .zip.
• Release nnunetv2-neuropoly to PyPI instead of Test PyPI
• Update totalspineseg to use nnunetv2-neuropoly.
• Update the SCT PR to remove all of the "hacky" fixes.
• Update the SCT PR to import the trainer class from the model files.

[plbenveniste]

I have done the following modifications:

• changed name back to nnunet-neuropoly
• removed the trainer specific to the lesion_ms model
• pushed version 2.6.2.dev3 to Test PyPI

@joshuacwnewton Do you want me to push to PyPI or would you rather do it yourself?

[joshuacwnewton]

I think maybe we should wait to push to PyPI until we've fully tested and approved the main SCT PR? Since if we make a mistake or need to revert something, the consequences are much more costly on the main PyPI (disruptions etc.).

[plbenveniste]

I think maybe we should wait to push to PyPI until we've fully tested and approved the main SCT PR? Since if we make a mistake or need to revert something, the consequences are much more costly on the main PyPI (disruptions etc.).

Ok good to know! I am almost pushed it PyPi and changed my mind at the very last minute. 😆

[plbenveniste]

@joshuacwnewton What should be done to move forward with this? I would like to merge the dev branch in SCT for the new MS lesion model soon

[joshuacwnewton]

Thank you for the ping, and my apologies for falling behind on this task.

• Now that you have added the trainer_class behavior to SCT, and you have gone through the steps of putting the trainer class in the model .zip (and our existing test for lesion_ms had passed), I think it should be OK to build and push our wheel of nnunetv2-neuropoly to the real PyPI. (I'll do this shortly.)
  • I need to clean up this branch a little to make sure we only have the necessary commits.
  • Then I need to take the changes on this branch and copy them over to the necessary version tags that we wish to have for our fork. (EDIT: Done! 2.6.2.patch)
  • Done! Release: https://pypi.org/project/nnunetv2-neuropoly/2.6.2
• Once this is done, we need to update totalspineseg to use our fork instead of nnunetv2 and push a new release.
• Once that is done, we can remove all of the "hacks" that we have added to the SCT side of things (e.g. install_sct), and instead simply specify nnunetv2-neuropoly in SCT's requirements.txt file.
• Once that's all done, your PR can be properly reviewed and merged like normal.