WIP: SpectralGPT for multi-temporal change detection

[SebastianGer]

Sets SpectralGPT encoder to mono-temporal if UperNetCD is used. Otherwise, SpectralGPT expects bitemporal input, but the UperNetCD purposefully splits the bi-temporal input into two mono-temporal images, to compute two separate image feature vectors and then compute the difference between for change detection.

Small issue: I need to change the "multitemporal" value of the encoder, depending on the chosen segmentor architecture. The best spot I found for that is in run.py before the encoder is initialized, which is kind of ugly. Any suggestion where to put it or how to handle this issue differently?

[KerekesDavid]

Some config values that depend on other config values are set under utils/configs.py.

But in general the pattern of doing something based on a name is not very nice, I'd much rather introduce some property in the segmentor config that then we can take into account (either by using it instead of the multitemporal flag, or setting the multitemporal flag based on it.)

[SebastianGer]

Some config values that depend on other config values are set under utils/configs.py.

Those mostly seem to move values from one dict into another, I don't think that the branching we need here is similar to that, so I'd put it into a new function.

But in general the pattern of doing something based on a name is not very nice, I'd much rather introduce some property in the segmentor config that then we can take into account (either by using it instead of the multitemporal flag, or setting the multitemporal flag based on it.)

We can use the existing task_name in the segmentor config and check if encoder.encoder_name=='SpectralGPT' and segmentor.task_name=='change-detection' and encoder.multi_temporal > 1, but the problem remains that this is needed during the encoder init, where we don't have access to the segmentor right now.

How about adding a function that gets called right after load_config, which ensures that encoder and segmentor are compatible and makes this change to the config before anything is initialized? ensure_compatible_configs or something, and it sits in utils/configs.py

[KerekesDavid]

Just have a separate encoder config file and load that in your run config when needed.

The only difference in the encoder config would be the multitemporal argument, but currently that would be overwritten when the config is loaded: https://github.com/yurujaja/geofm-bench/blob/906d8c161994cec8c3b73a12f75781a1f02ffca5/utils/configs.py#L48

Have the segmentor config contain a split_output=True setting, and make the encoder act on that. This requires a separate segmentor config, but not a separate encoder config.

SpectralGPT sets its patch embedding and positional embedding based on multispectral during init, so we can't just pass it during runtime. If we want to provide split_output during init, then we have to start passing the segmentor cfg to all encoders and change all of their code accordingly for that. I assumed that changing all encoders would be less desirable than adding a function to change the config, but I can do that.

We could easily introduce a functionality where the parameters specified in the run config override sub-configs, so you can just write encoder.multitemporal=True in the run config itself and be done with it.

In that case, every user that wants to run change detection with xView2 will probably first run into the error message I got, and then hopefully know to add this to the run config, right? Philosophically, I'd prefer to take that responsibility from the user by handling the special case explicitly in code, especially because it's a structural problem that we know about, not a very special case that they can't expect to be handled by the code.

tldr: I like my approach :D But I can switch to the second idea, if you prefer that.

[yurujaja]

We refer to the original spectralgpt model parameters where the temporal dimension is set to 1 as the model is not pretrained on time series data. Accordingly, we also set the temporal dimension to 1. I modify the corresponding changes in MTUperNet forward functions.

[SebastianGer]

We refer to the original spectralgpt model parameters where the temporal dimension is set to 1 as the model is not pretrained on time series data. Accordingly, we also set the temporal dimension to 1. I modify the corresponding changes in MTUperNet forward functions.

With this, we only need to handle the situations in the UperNet forwards. I removed my previously suggested way of handling the problem. Now we just ensure that the temporal dimension exists when we pass an image to SpectralGPT. We have the information that we're passing to SpectralGPT in UperNet, because the segmentor always knows its encoder, so no additional changes are necessary. Much cleaner solution, thanks Yuru.

And sorry David for apparently editing your comment instead of replying to it. I was not aware that that's even an option, but apparently I clicked it.