Skip to content

Using float for time in the Parcels kernel #2117

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Using float for time in the Parcels kernel #2117

wants to merge 4 commits into from

Conversation

@erikvansebille
Copy link
Member

@erikvansebille erikvansebille commented Jul 28, 2025

This PR reverts to using float inside the kernel (as in v3), as performance tests have shown that this is approx 50% faster. See e.g. Parcels-code/parcels-benchmarks#1 (comment)

  • Chose the correct base branch (main for v3 changes, v4-dev for v4 changes)
  • Added tests

@erikvansebille
Using float for time in the Parcles kernel
d14dadc 
As performance tests have shown that this is approx 50% faster
@VeckoTheGecko VeckoTheGecko mentioned this pull request Jul 28, 2025
Closed
1 task
@erikvansebille
Copy link
Member Author

erikvansebille commented Jul 28, 2025

Note that in this PR< we still assume that the user inputs particle.time, endtime, runtime, dt etc as np.datetime64 or np.timedelta64. This requires quite a bit of type-conversion under the hood. The code would het simpler if we expect (all/most of) these inputs to be floats too. Not sure if that's worth it, though

VeckoTheGecko
VeckoTheGecko requested changes Jul 28, 2025
View reviewed changes
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think we need to adjust some things - not sure if that has to be in this PR though, happy for it to come in a different one

@github-project-automation github-project-automation bot moved this from Backlog to Ready in Parcels development Jul 28, 2025
@VeckoTheGecko VeckoTheGecko mentioned this pull request Jul 28, 2025
Merged
3 tasks
@VeckoTheGecko
Update release time time_interval comparison, add naming and test stubs
06e3098 
Small correctness fix. Otherwise could have run into edge cases with multiple fields defined on different (yet compatible) calendars.
@erikvansebille
Copy link
Member Author

erikvansebille commented Sep 2, 2025

We're not pursuing this anymore; getting good performance with numpy.datetime

@github-project-automation github-project-automation bot moved this from Ready to Done in Parcels development Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VeckoTheGecko VeckoTheGecko VeckoTheGecko approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Parcels development
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@erikvansebille @VeckoTheGecko