[BUG] SentinelHubInputTask downloads incorrect timestamps about eo-learn HOT 6 CLOSED

Hi @ColinMoldenhauer,
this is a tricky one, but the tl;dr would be "These two should be separate parameters but are not in order to limit number of parameters."

So on one hand we use time_difference when doing searches in the catalog. This one is simple enough and it's what you have in mind when working with the parameter. However this also specifies the "mosaicking time interval" for the Sentinel Hub.

Neighbouring tiles often have slightly different sensing times. So requesting the data for a very specific time, if your BBox is on the border of two tiles, you might just get the data from one of them. However SentinelHub does mosaicking for us, provided we give him enough of a time interval. As you can imagine a 2 week interval can result in funky results :D

When the time_difference is small enough (a couple hours at most) these two can be linked with little issue. But in your case, where the time_difference is large, the coupling makes no sense.

However with the last update that we did, we can now pass in custom filtration functions! So we do have a way of decoupling these two parameters (though a bit more work is needed). My suggestion would be that you set the time_difference to something smaller (5 minutes or so if you need a very precise timestamp, maybe 2 hours if you don't need time precision but can't afford to lose data on technicalities). Think of this as mosaicking_interval. And then add the 14 days as part of the custom filtration function (hardcode it, or add it as a parameter and use functools.partial when constructing task.

This way your filtration will take care of the 14-day steps, and the 2 hour time_difference will make sure the images you get back are not affected by small time differences of tile acquisition.

If this wont suffice we can see if something else can be done, but I'd try to avoid introducing even more parameters. The core of the issue is that SentinelHubInputTask tries to be super user friendly and reduces the number of things to take care of, but this makes it less flexible for power-users. We are considering expanding the arsenal of SH tasks, but it will likely come a bit down the road.

from eo-learn.

Thank you for the in-depth answer, I didn't consider the mosaicking! I totally agree that the custom filtration method will allow a workaround in order to acquire the correct data corresponding the correct timestamps. However, in the case of default behavior, I still think that the meta-info EOPatch.timestamp should under no circumstances differ so crucially from the actual timestamp of the downloaded and displayed data as in the above example (10 days difference!).

As a compromise that does not affect usability of the Task, some form of warning of unexpected behavior would be nice, even if it's only in the docstring :)

from eo-learn.

However with the last update that we did, we can now pass in custom filtration functions! So we do have a way of decoupling these two parameters (though a bit more work is needed). My suggestion would be that you set the time_difference to something smaller (5 minutes or so if you need a very precise timestamp, maybe 2 hours if you don't need time precision but can't afford to lose data on technicalities). Think of this as mosaicking_interval. And then add the 14 days as part of the custom filtration function (hardcode it, or add it as a parameter and use functools.partial when constructing task.

This way your filtration will take care of the 14-day steps, and the 2 hour time_difference will make sure the images you get back are not affected by small time differences of tile acquisition.

Maybe I am wrong, but shouldn't timestamps be the only input argument to the filtering function then? Currently, the custom filter must be of the form

def my_filter(timestamps, time_difference): pass

If the default filtering function sentinelhub.filter_times() is used, functool.partial would need to be used in order to pass self.time_difference.

from eo-learn.

Maybe I am wrong, but shouldn't timestamps be the only input argument to the filtering function then? Currently, the custom filter must be of the form

def my_filter(timestamps, time_difference): pass

If the default filtering function sentinelhub.filter_times() is used, functool.partial would need to be used in order to pass self.time_difference.

I do agree that that might be the more appropriate signature. But then the signature of get_available_timestamps would change in a code-breaking way. But perhaps you're right and it's time to break some code 🤔

from eo-learn.

Fair point. Maybe the signature of get_available_timestamps can remain unchanged for now, maybe with a deprecation warning added to the code?

from eo-learn.

We extended the docstring to warn about using large values, which we consider sufficient for now. The sentinelhub tasks should be getting a much needed overhaul soon.

from eo-learn.