Skip to content

Conversation

@VeckoTheGecko
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko commented Aug 5, 2025

Changes:

  • Cleares up Variable and Particle classes, making the dictinction between a ParticleClass (a type of Particle), and a KernelParticle (an accessor class used to read and set particle data in the arrays). This clarification allows for clearer abstractions and consolidation of responsibility (helping with testing).
  • Removed lonlatdepth_dtype from particleset init
    • Created a public helper function get_default_particle(dtype: np.float32 | np.float64) which allows users to create particles with the right precision from the beginning (rather than redefining after the fact).
  • Consolidate particle data creation to a function create_particle_data that works from the particle class, ngrids, time_interval, initial
  • Remove MPI related code from ParticleFile, and some cleanup of the ParticleFile class (particlefile remains untested at the moment - more changes will come in a future PR)

Would be good to also discuss variable naming - let me know if you have any suggestions.

  • Chose the correct base branch (v4-dev for v4 changes)
  • Fixes #
  • Added tests

…assert .zarr file extension

MPI is not yet supported in v4-dev anyway, and MPI will likely be implemented differently (#2074).
This makes it distinct what a particle is on the "particle type" level (defining what attributes exist etc) and on the kernel level (i.e., just a link to the particle data.

Naming is a WIP
This allows users to get a default particle of differing spatial precision. This replaces the argument `lonlatdepth_dtype` that was previously used on the ParticleSet init.
Make it explicitly an initial dict rather than as kwargs
Not sure why this is necessary since I only refactored the way the particle data was created?
Copy link
Member

Choose a reason for hiding this comment

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

So the code (see below) to temporarily set time to np.nat until we know the sign of dt in execute() stays as it is?
https://github.com/OceanParcels/Parcels/blob/30aec22d2939e285bfbd44f6617e36019002f6d3/parcels/particleset.py#L100-L103
and
https://github.com/OceanParcels/Parcels/blob/30aec22d2939e285bfbd44f6617e36019002f6d3/parcels/particleset.py#L536-L540

That would be fine with me; but it could perhaps be a bit more transparently coded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, something to keep in mind when we refactor the particleset execute loop

@VeckoTheGecko VeckoTheGecko force-pushed the particle-particledata branch from 0afb92a to 6d8bbce Compare August 6, 2025 11:34
@VeckoTheGecko VeckoTheGecko mentioned this pull request Aug 6, 2025
3 tasks
@VeckoTheGecko VeckoTheGecko force-pushed the particle-particledata branch from 6d8bbce to ab67bd4 Compare August 6, 2025 11:40
erikvansebille added a commit that referenced this pull request Aug 6, 2025
@VeckoTheGecko
Copy link
Contributor Author

@erikvansebille after syncing this branch with v4-dev, I am getting some strange infinite loop bugs in the following tests to do with the RK45 kernel ..... I'm not really sure how to go about solving them at the moment, perhaps if you have a second tomorrow we can look through together since you're familiar with the rk45 kernel? (or can pytest.skip them for now until we have time to work through this. I think the pfile stuff needs to remain my priority for now

pytest 'tests/v4/test_advection.py::test_moving_eddy[RK45-1e-05]' 'tests/v4/test_advection.py::test_decaying_moving_eddy[RK45-1e-05]' 'tests/v4/test_advection.py::test_gyre_flowfields[stommel_gyre-A-RK45-1]' 'tests/v4/test_advection.py::test_gyre_flowfields[peninsula-A-RK45-1]'

VeckoTheGecko and others added 3 commits August 6, 2025 18:06
For some reason RK45 was set so strict that it reduced dt to 1 second, severely slowing down the tests
Local environment was desynced with CI - accidentally included zarr v3 code. Let's stick to v2 for now then upgrade later
@VeckoTheGecko VeckoTheGecko merged commit 9c6ce08 into v4-dev Aug 7, 2025
9 checks passed
@VeckoTheGecko VeckoTheGecko deleted the particle-particledata branch August 7, 2025 08:59
@github-project-automation github-project-automation bot moved this from Backlog to Done in Parcels development Aug 7, 2025
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.

3 participants