Rename particle position attributes from "lon" and "lat" to "x" and "y"#2719
Merged
Merged
Conversation
Contributor
Author
|
Also note that I haven't updated the docs as these are currently broken due to the last refactoring. We should fix them in a separate PR. |
erikvansebille
left a comment
erikvansebille
left a comment
Member
There was a problem hiding this comment.
Code changes look good; see below a few minor comments
Two more important ones
- Should we also change
particles.timetoparticles.t, for symmetry with the other one-letter attributes? - Would it be helpful to users to provide some informative error message if they create a ParticleSet or Kernel with
lon, informing them that this has changed in v4? Or do we expect them to just read the migration guide? I guess that this will be one of the most impactful changes for users wanting to port over their code (along with the vectorization of the particleset).
| - Users need to explicitly use `convert_z_to_sigma_croco` in sampling kernels (such as the `AdvectionRK4_3D_CROCO` or `SampleOMegaCroco` kernels) when working with CROCO data, as the automatic conversion from depth to sigma grids under the hood has been removed. | ||
| - We added a new AdvectionRK2 Kernel. The AdvectionRK4 kernel is still available, but RK2 is now the recommended default advection scheme as it is faster while the accuracy is comparable for most applications. See also the Choosing an integration method tutorial. | ||
| - Functions shouldn't be converted to Kernels before adding to a pset.execute() call. Instead, simply pass the function(s) as a list to pset.execute(). | ||
| - Kernel variables `lon` and `lat` have been renamed to `x` and `y`, and `dlon` and `dlat` have been renamed to `dx` and `dy`. These changes are also reflected on the ParticleSet as well as the particlefile output. |
Member
There was a problem hiding this comment.
Should we also update time to t?
Because a user would now sample by fieldset.field[particles.time, particles.z, particles.y, particles.x]. This could be more 'symmetric' with
fieldset.field[particles.t, particles.z, particles.y, particles.x]
Contributor
Author
That's a good idea. Added to #2109 |
erikvansebille
approved these changes
Jul 6, 2026
Contributor
Author
OK, let's do that. Tracking in a separate issue to be done in another PR. |
Update particle definition and all related code - particle.py particle variables - ParticleSet args - all internal mentions of .lon and .lat - test suite code
Co-authored-by: Erik van Sebille <e.vansebille@uu.nl>
VeckoTheGecko
force-pushed
the
push-klyynlpzyoqm
branch
from
62bdac2 to
98422b4
Compare
Contributor
Author
|
@erikvansebille merged now! |
erikvansebille
added a commit
to erikvansebille/Parcels
that referenced
this pull request
Jul 6, 2026
erikvansebille
added a commit
to erikvansebille/Parcels
that referenced
this pull request
Jul 6, 2026
erikvansebille
added a commit
to erikvansebille/Parcels
that referenced
this pull request
Jul 6, 2026
erikvansebille
added a commit
to erikvansebille/Parcels
that referenced
this pull request
Jul 6, 2026
erikvansebille
added a commit
to erikvansebille/Parcels
that referenced
this pull request
Jul 6, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Kernel variables
lonandlathave been renamed toxandy, anddlonanddlathave been renamed todxanddy. These changes are also reflected on the ParticleSet as well as the particlefile output.Changes:
This now means that users will need to use
xandywhen refering to particle positions. This includes in kernels, when defining the particlesets themselves, and when working with output after the face.Note that I haven't gone about updating local variable names in tests. I don't think it provides any benefit to do so (and for those tests using "lon" might be more informative than using "x" as it hints at the mesh type used).
Checklist
mainfor normal development,v3-supportfor v3 support)AI Disclosure
None used.