Skip to content

Support for writing ParticleSetView#2620

Merged
erikvansebille merged 7 commits into
Parcels-code:mainfrom
erikvansebille:writing_in_kernel
May 21, 2026
Merged

Support for writing ParticleSetView#2620
erikvansebille merged 7 commits into
Parcels-code:mainfrom
erikvansebille:writing_in_kernel

Conversation

@erikvansebille
Copy link
Copy Markdown
Member

Description

This PR implements a way to write a ParticleSetView into a ParticleFile, and adds a tutorial to show how it works. I think this could be a killer feature, as it provides the user much more control over when particles are written (an often-asked feature pre-v4!)

Note that there are still a few rough edges about this implementation. Notably:

  1. Since the FieldSet is not an attribute of a ParticleSet view (unlike a ParticleSet), it needs to be added as an extra argument. An alternative could be to also add a ParticleSetView.fieldset?
  2. The ParticleFile.write() requires a time to write, but the Kernel itself doesn't know the timestep. Users can provide particles.time, but this may result in particles being written that have not yet started (to be explored)
  3. A particle file that is intended to only be written in a kernel (so not in the pset.execute loop) still requires an outputdt argument on initialisation, even though that has no effect. Would be better to also allow None here?
  4. A particlefile that is only used in the kernel does not get closed automatically, so needs to be closed manually. Not sure though if there's anything we can do about that.

Once we settled on these four open questions here, I will add unit tests etc

Checklist

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.
    • Describe how you used it (e.g., by pasting your prompt):

@erikvansebille
Copy link
Copy Markdown
Member Author

@VeckoTheGecko, keen to hear your thoughts about these four items above!

@VeckoTheGecko
Copy link
Copy Markdown
Contributor

Sorry for the delay in reviewing this. I think that its a good feature to have. I am wondering about how it interacts with the rest of the codebase and how we use Particle file (ideally how does testing of this look like, and what is the robustness of that testing?)

This bigger picture isn't something I have at the moment, but also isn't something I think that we should wait on either - I just wanted to flag it. I hope to come back and revisit it.

I'm happy to merge this as is. I think the tutorial running in CI is sufficient for testing for now as a first iteration?

Also, do you want to flag this as an experimental feature? (e..g, by adding a callout block in the tutorial saying such) I'm happy to leave that up to your discretion if that's ok - you have more of a feel for this feature than I do.

@erikvansebille
Copy link
Copy Markdown
Member Author

Thanks for the feedback, @VeckoTheGecko! I will then indeed add a flag to warn this is experimental in the tutorial.

One question though: do you think we could/should add the fieldset as an attribute to the ParticleSetView too? It will make the API and the .write() code even simpler for the user, but of course adds more overhead to the (private) ParticleSetView class.

Or would you want me to implement this first, before you can assess what works better?

@VeckoTheGecko
Copy link
Copy Markdown
Contributor

One question though: do you think we could/should add the fieldset as an attribute to the ParticleSetView too? It will make the API and the .write() code even simpler for the user, but of course adds more overhead to the (private) ParticleSetView class.

I think having it as an argument to .write (as it is) is good for now. AFAICT the fieldset is only used for getting the time interval information - in future I think we might want to refactor to give the ParticleFile access to the time interval information when the pset.execute loop spins up so that we don't need to worry about this. I think that would be the cleanest way of handling this, though I would rather leave that for a future PR

I wouldn't add it to the ParticleSetView class

@erikvansebille erikvansebille marked this pull request as ready for review May 21, 2026 06:41
@erikvansebille erikvansebille merged commit 929b735 into Parcels-code:main May 21, 2026
16 of 17 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Parcels development May 21, 2026
@erikvansebille erikvansebille deleted the writing_in_kernel branch May 21, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Explore and expose use of writing particles in a Kernel

2 participants