Skip to content

Conversation

@erikvansebille
Copy link
Member

While the Morton encoding for the curvilinear search (#2175) is very fast, it turns out to be even faster to use the element indices (particle.ei) in the search.

Using the benchmark script in OceanParcels/parcels-benchmarks@9a39630 (#5), we get the following speed-up:

Running 4_999_696 particles with parcels v4
Execution time 1: 44 seconds
Execution time 2: 1 seconds

where execution 1 is with the Morton search (since particle.ei = None) and execution 2 is with the code in this PR to use the particle.ei.

Note that this is the most optimal case where particles don't move between searches, but that is actually very often the case because

  1. A Kernel loop requires multiple Field interpolations at the same location
  2. Most particles move less than one grid cell between Kernel loops

@erikvansebille
Copy link
Member Author

@fluidnumerics-joe, there's a failing unit test in the uxgrid part:

FAILED tests/v4/test_particleset_execute.py::test_uxstommelgyre_pset_execute - KeyError: 'FACE'

with detailed error

parcels/particleset.py:543: in execute
    self._kernel.execute(self, endtime=next_time, dt=dt)
parcels/kernel.py:261: in execute
    f(pset[evaluate_particles], self._fieldset)
parcels/application_kernels/advection.py:103: in AdvectionEE
    (u1, v1) = fieldset.UV[particles]
               ^^^^^^^^^^^^^^^^^^^^^^
parcels/field.py:338: in __getitem__
    return self.eval(key.time, key.depth, key.lat, key.lon, key)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
parcels/field.py:305: in eval
    position = self.grid.search(z, y, x, ei=_ei)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
parcels/uxgrid.py:91: in search
    bcoords = try_face(fi)
              ^^^^^^^^^^^^
parcels/uxgrid.py:77: in try_face
    bcoords, err = self._get_barycentric_coordinates_latlon(y, x, fid)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
parcels/uxgrid.py:117: in _get_barycentric_coordinates_latlon
    n_nodes = self.uxgrid.n_nodes_per_face[fi].to_numpy()
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
../../miniconda3/envs/parcels/lib/python3.13/site-packages/xarray/core/dataarray.py:902: in __getitem__
    return self._getitem_coord(key)
           ^^^^^^^^^^^^^^^^^^^^^^^^
../../miniconda3/envs/parcels/lib/python3.13/site-packages/xarray/core/dataarray.py:896: in _getitem_coord
    _, key, var = _get_virtual_variable(self._coords, key, dim_sizes)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Probably has to do with basegrid.ravel and .unravel now expecting an ndarray (since they are vectorized)

Can you see if you can fix this for uxgrid?

@fluidnumerics-joe
Copy link
Contributor

Taking a look :)

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.

Looks good! Some minor clarifications but otherwise LGTM

@VeckoTheGecko
Copy link
Contributor

Happy to merge when you're ready @erikvansebille :)

@erikvansebille erikvansebille merged commit 5079236 into v4-dev Sep 17, 2025
9 checks passed
@erikvansebille erikvansebille deleted the implement_ei_in_search branch September 17, 2025 12:26
@github-project-automation github-project-automation bot moved this from Backlog to Done in Parcels development Sep 17, 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.

4 participants