SlidingTrainer.validate_case() should compute metrics on full volume after sliding window reconstruction

[perctrix]

See #126.

[cloudflare-workers-and-pages]

Deploying mipcandy with    Cloudflare Pages

Latest commit:	7ad0c44
Status:	🚫  Build failed.

View logs

[ATATC]

@perctrix can you double-check my changes? I'll do an overall review later.

[ATATC]

Lowkey I already noticed the problem. Having a forward() method does not support multiple outputs from the model at all.

[ATATC]

Remove the forward() interface and merge it back into backward().

[ATATC]

Remove the forward() interface and merge it back into backward().

@perctrix Can you fix this?

[perctrix]

I'm working on that

[perctrix]

@ATATC I saw your changes and your comment.

A few questions:

1. What's the motivation for extracting forward() to the base class? The old design (model call inside backward) naturally supported multiple outputs.
2. Since you already noticed the problem, do you have a solution in mind?

[perctrix]

@ATATC Done. Removed the forward() interface and merged model call back into backward().

[ATATC]

@ATATC I saw your changes and your comment.

A few questions:

1. What's the motivation for extracting forward() to the base class? The old design (model call inside backward) naturally supported multiple outputs.
2. Since you already noticed the problem, do you have a solution in mind?

Cuz (toolbox.ema if toolbox.ema else toolbox.model)(image) doesn't often change and users will tend to forget about this. Separating forward calls and backward calls would make it more modular, but we would need to add types forward() -> torch.Tensor | Sequence[torch.Tensor] which would propagate to all other classes. I was trying to avoid such a massive change, so I suggested just stick with the current backward()-only design.

[ATATC]

@perctrix I have made some changes to the branch. I removed compute_metrics() and added back the validate_case_windowed() because SlidingTrainer should not assume single outputs for segmentation tasks.