Skip to content

Conversation

@erikvansebille
Copy link
Member

As discussed in #2130, using np.timedelta64 and np.datetime64 as core data structured for particles.dt and particles.time brings some major issues:

  1. Users will have to convert from timedelta to float inside each kernel, and use the float for multiplying with velocities
  2. There is a fundamental issue with uneven timedelta64 dt, see Issue with uneven dt and RK advection #2380

This PR therefore implements a revert to the v3 approach, where time and dt are floats throughout the Kernel. This also allows (later) integration with pint for unit-aware computations in the Kernel

During development, I also came across a bug in RK45 (fixed in 18a4590), which needs a particles.next_dt variable in order to be able to increase dt in the Kernel loop (otherwise, displacements are computed with the old dt, but the time-update is with the new dt).

And fixing most unit tests; one breaking unit test remaining
Note that I originally added in it 6a67411 (as part of
#2397) but then reverted it in e15c46c because I thought it wasn't necessary.

Now, it turns out it _is_necessary to have this next_dt when dt is allowed to increase
@erikvansebille erikvansebille marked this pull request as ready for review December 2, 2025 11:52
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko left a comment

Choose a reason for hiding this comment

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

Only managed to do a partial review today, looks good for the most part - will finish tomorrow. I went ahead and pushed the refactor e479387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants