Skip to content

Conversation

@j-atkins
Copy link
Collaborator

@j-atkins j-atkins commented Dec 9, 2025

Overview

This PR removes SpaceTimeRegion from the expedition config. Now, instead, space-time boundaries are defined in reference to the extremes of the waypoints when building fieldsets (i.e. the minimum/maximum latitude/longitude/time across all waypoints). This avoids divergence between user defined waypoints and the prescribed space-time region (i.e. #259).

I also experimented with not defining any spatial limits at all for the fieldset generation. For instruments which require data at depth (Argos, CTDs, XBTs, ADCP) this is still too slow/memory intensive, so I have kept the lat, lon subsetting in copernicusmarine.open_dataset(..). However, for surface-only instruments (Drifters), the lat, lon subsetting can be relaxed. This is a big advantage because it removes out-of-bounds issues for Drifters. The bathymetry fields also require no lat, lon bounds.

Otherwise of interest - and happily - I think there could a speed boost as result of the changes in this PR (with e.g. Argo floats and the underway instruments), but I haven't confirmed this nor diagnosed exactly why this might be happening.

Drifter/Argo lifetimes

Drifters

  • The logic for determining the temporal extent for drifter fieldsets has changed. Before there was a generic 41+21 day time buffer added.
  • Now, the drifter lifetime parameter (in InstrumentConfig) also controls the endtime of the fieldset (so it will be the last waypoint time + the drifter lifetime).
  • This, in combination the relaxed lat, lon bounds, means that users can set whatever lifetime they wish without hitting out-of-bounds errors.

Argo floats

  • Previously Argo float lifetimes were controlled by the temporal extent of the fieldset.
  • There is a now a lifetime parameter added to the Argo configuration (in InstrumentConfig), giving more control to the user. This defaults to 3 months.
  • Now, similarly to Drifters, the fieldset endtime is controlled by the lifetime parameter, rather than the other way round.
  • Because we cannot yet relax the lat, lon bounds for the Argos, it is still possible to hit out-of-bounds errors with Argos. Though, this is not very common and I think less likely than was previously the case with Drifters at the surface.
  • Parcels v4 could allow us to relax the lat, lon bounds on all instruments, so this may not even be problem for Argos in the future (?)

@j-atkins j-atkins marked this pull request as draft December 9, 2025 08:42
@j-atkins j-atkins marked this pull request as ready for review December 9, 2025 11:24
Copy link
Member

@erikvansebille erikvansebille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Great improvement which will reduce annoying outofbounds errors and thereby increase authenticity!

A few comments below

{"name": "cycle_days"},
{"name": "drift_days"},
{"name": "stationkeeping_time", "minutes": True},
{"name": "lifetime", "minutes": True},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what "minutes' here means, but would this not be more logical in days?

Comment on lines 160 to 165
minimum_longitude=self.min_lon - latlon_buffer
if spatial_constraint
else None,
maximum_longitude=self.max_lon + latlon_buffer
if spatial_constraint
else None,
minimum_latitude=self.min_lat - latlon_buffer
if spatial_constraint
else None,
maximum_latitude=self.max_lat + latlon_buffer
if spatial_constraint
else None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct python code? Looks strange, without indents etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agree it doesn't look great, ruff auto-formatted to this. I'll adjust it anyway 👍


@pydantic.field_serializer("lifetime")
def _serialize_lifetime(self, value: timedelta, _info):
return value.total_seconds() / 60.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, would this not make more sense in days?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice that users don't need to set a pace_time_region anymore!

min_depth_meter: 0.0
vertical_speed_meter_per_second: -0.1
stationkeeping_time_minutes: 20.0
lifetime_minutes: 90720.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in days?

Comment on lines +123 to +124
schedule_start=datetime.datetime(1995, 6, 1, 0, 0, 0),
schedule_end=datetime.datetime(1995, 6, 30, 0, 0, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps not relevant for this PR< but at some point it might be good to transition to numpy.datimetime64 throughout the code in order to better align with Parcels v4?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SpaceTimeRegion problems when changing waypoints after initialisation

3 participants