ParticleFile refactor and re-integration into v4-dev #2142
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VeckoTheGecko
force-pushed
the
pfile-rework
branch
from
e7fd808 to
64e09d8
Compare
VeckoTheGecko
force-pushed
the
pfile-rework
branch
3 times, most recently
from
0929bc5 to
7bb4f89
Compare
VeckoTheGecko
changed the title
VeckoTheGecko
commented
Sep 4, 2025
parcels/kernel.py
Outdated
Comment on lines
96
to
98
| def name(self): | ||
| return f"{self._ptype.name}{self.funcname}" | ||
| # return f"{self._ptype.name}{self.funcname}" # TODO v4: Should we propogate the name of the particle to the metadata? At the moment we don't have the concept of naming particles (somewhat incompatible with the .add_variable() API?) | ||
| return f"{self.funcname}" |
Contributor
Author
There was a problem hiding this comment.
^
Should we force users to name their particles @erikvansebille ? IIRC in v3 particles created with add_variable were called NewParticle?
Contributor
Author
|
Still need to fix the tests |
parcels/particle.py
Outdated
| Variable( | ||
| "time", | ||
| dtype=_SAME_AS_FIELDSET_TIME_INTERVAL.VALUE, | ||
| attrs={"long_name": "", "standard_name": "time", "units": "seconds", "axis": "T"}, |
Member
There was a problem hiding this comment.
Are the units here always in seconds? Does that not depend on the type of time?
Contributor
Author
There was a problem hiding this comment.
When serialized out, this gets updated to seconds since ... for datetime objects
This was referenced Sep 4, 2025
Setting the particle write status during execution is no longer supported
Also update all references of particledata to _data
Looking at time_nextloop instead. We might rework the kernel loop in future (and the responsibilities of time, time_nextloop, as well as their effect on particle writing).
Not sure how that got in there.... Typo
- Set metadata in file - Remove temp pytest marker
Also flag tests that should be looked at later - as they are out of scope for this PR (or may be able to be tested another way)
VeckoTheGecko
force-pushed
the
pfile-rework
branch
from
0856fa4 to
16032a4
Compare
In favour of directly using the ParticleFile object
erikvansebille
approved these changes
Sep 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
v4-devfor v4 changes)This PR brings back particlefile writing into the Parcels codebase, making it compatible with
v4-dev.Changes:
ParticleSetI think that this can be significantly improved in future (namely by working natively with zarr rather than intializing with xarray first then "extending" with zarr) making the code cleaner and more robust.
Previous description
@erikvansebille Just thought I would send you a quick update on my progress on the PariticleFile writing. PR is very much WIP (no review needed), and I will edit this description again once finalized.
pfile-native-zarr- this will make its way into this PR). I think we can work with zarr and extend the size when we encounter out of bounds. I still need to properly test this (and perhaps report a bug to Zarr) since I was encountering irregularities in the produced dataset using with the exact code I had (observations written at the end of the chunk and not the beginning).Bad news is that this has taken longer than I hoped. I was having some difficulty working with v3
test_particlefile.pyfile - since its more of an integration test suite, and there have been a lot of changes since branching off main, the errors weren't informative. I have now taken a step back to build up the test suite using unit tests and using that to guide refactoring - but ran out of time today.I'll get back to this once I'm back online.