-
Notifications
You must be signed in to change notification settings - Fork 168
Adjust longitudes for antimeridian cells to account for periodicity #2391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adjust longitudes for antimeridian cells to account for periodicity #2391
Conversation
The runtime was too short to reach the antimeridian Note that this unit test now throws an error ``` parcels._core.statuscodes.GridSearchingError: Grid searching failed at (z=[0.], lat=[87.97271], lon=[181.24731]) ```
|
Thanks for this patch, @fluidnumerics-joe; but it doesn't seem to work (yet). I've updated the runtime of the |
|
Note that the error thrown before was at |
|
The core assumption, noted in the comment of the changes, is that the particle position is less than 180, to match the way you were implementing periodic boundary conditions in the associated issue. What is the assumed range of particle longitudes going forward in parcels? |
|
This patch fixes the original issue reported where the particle longitude is -177.4 |
@erikvansebille and I were discussing what the assumed range should be in parcels, and although (-180,180) seems logical to me, we also considered allowing particles to have any lon, which could be mapped to the same location using a modulo. Although the patch is able to find the particle at |
|
Copy that. I'll get something more robust in place |
Just a note, the ERA5 data when downloaded straight from CMS is in the range [0,360). I'd vote for lon |
* Particle longitude and grid longitudes are re-mapped to [-180,180) inside the curvilinear particle in cell check method. * Depending on the proximity of the particle to +/- 180, cell corners are adjusted to the image position that is closest to the particle.
By not requiring periodicBC kernel anymore
|
Thanks @fluidnumerics-joe, for fixing this bug! I just committed e24c631, which extends the unit test for curvilinear grids to much longer runtime (and thus multiple loops); this also required a small bugfix to the interpolator which makes it more robust now |
erikvansebille
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy that with this PR, periodic boundaries on spherical grids should work out-of-the-box, without users having to worry about them
Resolves #2383