Migrate to Parcels v4 - #342
Conversation
…ument column entries are consistent
for more information, see https://pre-commit.ci
* docs: copy updates from old/broken branch * Update depth dimension attribute explanation
…able tmp ds writing without heavy RAM spikes
…ting and "simulating" print
|
This PR is ready for re-review. Please note a couple of things, which have been added more recently, including tinkering around Also, there are some changes that have been reviewed already, most notably the updates to the docs, and probably don’t need much time spent on them. Apologies this probably wasn’t the most efficient way of structuring the PRs. |
erikvansebille
left a comment
There was a problem hiding this comment.
Impressive effort, @j-atkins! See below some small comments/questions
| fieldsets_list.append(fs) | ||
|
|
||
| combined_fieldset = fieldsets_list[0] | ||
| for fs in fieldsets_list[1:]: | ||
| combined_fieldset = combined_fieldset + fs |
There was a problem hiding this comment.
Can you not simply keep adding the fieldsets? Instead of first making a list and then adding the entries of that list?
| def _sample_salinity(particle, fieldset, time): | ||
| particle.salinity = fieldset.S[time, particle.depth, particle.lat, particle.lon] | ||
| def _sample_salinity(particles, fieldset): | ||
| particles.salinity = fieldset.S[particles.t, particles.z, particles.y, particles.x] |
There was a problem hiding this comment.
In general, it's better to sample with [particles] since that retains the particles._ei element indices, so makes grid searching faster
| particles.salinity = fieldset.S[particles.t, particles.z, particles.y, particles.x] | |
| particles.salinity = fieldset.S[particles] |
Also relevant for the other sampling kernels
| particles_lowering.raising = np.where( | ||
| particles_lowering.z + particles_lowering.dz < particles_lowering.max_depth, | ||
| 1, | ||
| particles_lowering.raising, | ||
| ) |
There was a problem hiding this comment.
Not sure how you feel, but I tend to prefer boolean indexing (like in the Argo float kernel) over np.where(), as I find the boolean indexing easier to read
| x=[ctd.spacetime.location.lon for ctd in measurements], | ||
| y=[ctd.spacetime.location.lat for ctd in measurements], | ||
| z=[ctd.min_depth for ctd in measurements], | ||
| t=[np.datetime64(ctd.spacetime.time) for ctd in measurements], |
There was a problem hiding this comment.
why si this casting to datetime64 needed? Can't it be done without?
| try: | ||
| value = bathymetry_field.eval( | ||
| 0, # time | ||
| np.float64(0.0), # time |
There was a problem hiding this comment.
Interesting that you needed this change. Is this a bug in Parcels?
There was a problem hiding this comment.
perhaps let @VeckoTheGecko also double-check the pinnings in this pixi.toml?
This PR migrates to using Parcels v4 for VirtualShip.
Argo float bug [in kernel(s)?], immediately goes out of bounds after first timestep... likely depth convention related--from-data.zarroutput) [new PR]positive: downorpositive: upARGO_FLOATS: Enhance performance + no need to updatecycle_ageduring ascent?Closes #239, closes #232, closes #231, closes #323, closes #238