SwinUNETR refactored img_size parameter and removed checkpointing dep…

[john-zielke-snkeos]

Description

Make two changes for the SwinUNETR network:

• The image_size parameter does not seem to have an effect apart from checking shape compatibility in the beginning. This is now expressed in the docstring and the parameter will be deprecated in the future. Instead the shape compatibility is checked during the forward pass on the actual shape
• newer pytorch versions accept a parameter use_reentrant. The old default of True is deprecated in favor of True. This PR sets the parameter to true and therefore adopts the recommended value and removes the warning.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[wyli]

thanks, it looks good to me, merging for more integration tests.

[wyli]

/build

[john-zielke-snkeos]

Not quite to sure how to read the logs of the blossom-ci (showing ****** Result of stage V100-quick-PT210+CUDA122 is FAILURE ******), but it ends in:

2023-10-06T15:23:25.9290649Z [2023-10-06T15:22:45.909Z] .Finished test: test_correct_results_00 (tests.test_adjust_contrast.TestAdjustContrast) (0.0112s)
2023-10-06T15:23:25.9290956Z [2023-10-06T15:22:45.909Z] Starting test: test_correct_results_01 (tests.test_adjust_contrast.TestAdjustContrast)...
2023-10-06T15:23:25.9291412Z [2023-10-06T15:22:45.909Z] [pt210-cuda122-jenkins-monai-premerge-2800-20tzh-6zk6r:2529 :0:2529] Caught signal 11 (Segmentation fault: address not mapped to object at address (nil))
2023-10-06T15:23:25.9291606Z [2023-10-06T15:22:46.164Z] ==== backtrace (tid: 2529) ====
2023-10-06T15:23:25.9291817Z [2023-10-06T15:22:46.165Z] 0 0x0000000000042520 __sigaction() ???:0
2023-10-06T15:23:25.9292000Z [2023-10-06T15:22:46.165Z] =================================
2023-10-06T15:23:25.9292394Z [2023-10-06T15:22:46.420Z] ./runtests.sh: line 700: 2529 Segmentation fault (core dumped) ${cmdPrefix}${cmd} ./tests/runner.py -p "^(?!test_integration).*(?<!_dist)$"

I'm not quite sure how this could be related to this PR. Could this be an issue with pytorch 2.1.0?

[john-zielke-snkeos]

One point regarding the image_size parameter deprecation: It's kind of weird to deprecate the first positional parameter. Since you cannot remove the parameter in your project code until we actually remove it, you cannot get rid of the warning until version 1.5. And from that version on the parameter will disappear, and if you did not change your code, the in_channels parameter will now have the value of the img_size parameter. I do not really see a nice way around this, only possible help could be to check if the depths parameter is a single int instead of a Sequence, and then warn of this issue.
Is there any best practice on how to do this? In my opinion, it might make sense to enforce keyword-argument only parameters for most networks and other functions where it is not totally obvious what the positional parameter would be (e.g. the first-parameter of convert_to_tensor). This way deprecating these parameters would be much easier and less error-prone.

[wyli]

/build

[wyli]

yeah, I also feel the initial implementation of requiring image_size as the first argument is a bit restrictive. the current way of deprecating it looks good to me...

[john-zielke-snkeos]

Should I create an issue to discuss moving all networks and other reasonable functions to accept only keyword-only arguments?