Enable any resolution for Unet

[qubvel]

What does this PR do?

1. Replace scale_factor with size argument in Unet DecodeBlock to enable forward with any image resolution (not only divisible by 32)
2. Improve Unet docstring, add example
3. Fix type hint for models (was bugged by double inheritance with HubMixin)
4. Use inference_mode instead of no_grad in tests

[codecov]

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...gmentation_models_pytorch/decoders/unet/decoder.py	96.29%	1 Missing ⚠️

Files with missing lines	Coverage Δ
segmentation_models_pytorch/base/model.py	81.57% <100.00%> (+2.79%)	⬆️
segmentation_models_pytorch/decoders/unet/model.py	100.00% <100.00%> (ø)
...gmentation_models_pytorch/decoders/unet/decoder.py	91.37% <96.29%> (+0.99%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[adamjstewart]

Not sure if I'm familiar enough with the details of the model to properly review this

[JulienMaille]

Could this break compatibility with onnx export -> OpenCV inference?

[qubvel]

@JulienMaille thanks for looking into it!

Right, initially, there was an issue with ONNX export, but nowadays ONNX export works with the size argument as well. I will check this out manually, but I plan to add ONNX exportability tests for all models.

[qubvel]

@JulienMaille I just checked this; the minimum opset to export the model with size is 11, while currently, the latest one is 21, so we are good to go

[adamjstewart]

Is this an intentional backwards-incompatible change? It is no longer possible to use U-Net without specifying height/width. Could we instead default to the same height/width as the input like we previously did?

[qubvel]

Hmm, not sure I got this.. this is just for a layer, but Decoder pass height and width. Can you please specify what is broken?

[adamjstewart]

Ah, the problem was that I'm directly using smp.decoders.unet.decoder.UnetDecoder. I got this failure in CI when I tried the main branch of SMP: https://github.com/microsoft/torchgeo/actions/runs/14042455085/job/39315632706?pr=2669. Perhaps we are relying on some undocumented implementation details in https://github.com/microsoft/torchgeo/blob/main/torchgeo/models/fcsiam.py?

[qubvel]

Part of the fails are related to modified decoder forward, previously input features were unpacked with *features while passing into the decoder's forward, but now the star is removed and they should be provided as list

[qubvel]

Removing star here should work

https://github.com/microsoft/torchgeo/blob/091ad0927c901dd750257a3cbddb755a6ae95dee/torchgeo/models/fcsiam.py#L114

And

https://github.com/microsoft/torchgeo/blob/091ad0927c901dd750257a3cbddb755a6ae95dee/torchgeo/models/fcsiam.py#L153

[qubvel]

The rest can be resolved with renaming center->add_center_block here
https://github.com/microsoft/torchgeo/blob/091ad0927c901dd750257a3cbddb755a6ae95dee/torchgeo/models/fcsiam.py#L83

[adamjstewart]

I know it's easy to support the new API, although it's harder to support both. I guess the question is whether this is intentional or not. Maybe it would help to add a "backwards-incompatible" label to PRs like this. Even better would be to deprecate the old syntax before completely removing it.

[qubvel]

Yes that was intentional, added in PR which fix support of torch.script/compile/export. These are internal modules, so I hope it will not break too many things.. But you are right, I will add the label and I will also highlight it in release notes