Update underway - #360
Conversation
… over from utils), change kernels to special-case underway kernels
…ggered for final instrument child classes
erikvansebille
left a comment
erikvansebille
left a comment
There was a problem hiding this comment.
Looks good; a few questions/suggestions below
| _time_ref_key = next(iter(self.variables)) | ||
| _time_ref_field = getattr(fieldset, _time_ref_key) | ||
| fieldset_starttime = _time_ref_field.data.time.isel(time=0).values |
There was a problem hiding this comment.
Could you also use fieldset.time_interval.left here?
|
|
||
| @dataclass(frozen=True) | ||
| class UnderwayCoordinates: | ||
| """1D evaluation grid arrays for underway instruments.""" |
There was a problem hiding this comment.
what do you mean with "grid arrays" here?
There was a problem hiding this comment.
Yes, perhaps this is confusing, will update
| "y": pa.array(coords.lats.astype(np.float32)), | ||
| "x": pa.array(coords.lons.astype(np.float32)), | ||
| "particle_id": pa.array( | ||
| np.arange(n, dtype=np.int64) |
There was a problem hiding this comment.
Would it not make sense to give all data the same particle ID (interpreting the ship as one particle)?
| origin_str = str(fieldset_time_origin).replace("T", " ") | ||
| t_metadata = {"units": f"seconds since {origin_str}", "calendar": "standard"} | ||
|
|
||
| # base schema mirroring Parcels ParticleFile schema, not yet with sampled variables |
There was a problem hiding this comment.
Would it make sense to also open a parcels.ParticleFile object to check if the schemes are the same - and error out if they are not? In that way, we can spot when the schemas start to deviate
There was a problem hiding this comment.
I have now added a new unit test for this (in ccd0ae3). The scheme is not in the public API for parcels.PartileFile so I have done it by executing a simple Parcels simulation and comparing the scheme of the resultant output against the UnderwayInstrument output.
|
Thanks for the comments @erikvansebille, I have now made the changes in response 👍 |
erikvansebille
left a comment
erikvansebille
left a comment
There was a problem hiding this comment.
Three small points/questions; looks good otherwise!
Co-authored-by: Erik van Sebille <erikvansebille@gmail.com>
2 participants
This PR implements a move to using
Field.evalfor underway instrument simulations (ADCP, UnderwaterST). This is enabled by Parcels v4 and delivers much faster performance.To facilitate this, there is a new abstract/intermediate class for underway instruments in base.py:
UnderwayInstrument, which the ADCP and UnderwaterST concrete subclasses inherit from. Plus a newUnderwayCoordinatesdataclass for housing the final sampling locations and timings. There are new tests for these in test_base.pyAlso, underway instruments still have 'kernels' but are not traditional Parcels kernels. Instead, they are standard functions which make the
Field.evalcall. This means underway instruments continue to meet the requirements for all instruments to have sampling kernels associated with sensors.Closes #231