Skip to content

Fix silent fail on pset sampling when time is NaN #2823

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

Merged
erikvansebille merged 4 commits into from
Aug 17, 2026
Merged

Fix silent fail on pset sampling when time is NaN #2823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c564544
Adding failing unit test
erikvansebille Aug 14, 2026
d21ae89
Raising error when time is NaN on field eval
erikvansebille Aug 14, 2026
86832b7
Add note to sampling tutorial
erikvansebille Aug 14, 2026
a86d3f0
Merge branch 'main' into fix_pset_sampling_without_time
erikvansebille Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/user_guide/examples/tutorial_sampling.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,15 @@
")"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"```{note}\n",
"Note that you need to explicitly set the `t` values in the ParticleSet; otherwise you will get a `ValueError: Time values cannot be NaN.` error. This is because (if not specified) the time values are NaN until they get set on `pset.execute()`, when the sign of `dt` is known.\n",
"```"
]
},
{
"attachments": {},
"cell_type": "markdown",
Expand Down
3 changes: 3 additions & 0 deletions src/parcels/_core/field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ def _assert_same_time_interval(fields: Sequence[Field]) -> None:

def _get_positions(field: Field, t, z, y, x, particles, _ei) -> tuple[dict, dict]:
"""Initialize and populate particle_positions and grid_positions dictionaries"""
if np.any(np.isnan(t)):
nan_indices = np.where(np.isnan(t))[0]
raise ValueError(f"Time values for particles with indices {nan_indices} cannot be NaN.")
Comment on lines +396 to +398

@VeckoTheGecko VeckoTheGecko Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering whether this should be somewhere in the execute loop?

i.e., make sure that the time for each particle is set before we go into the main loop

Is there a usecase of t being nan during execution?

@erikvansebille erikvansebille Aug 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, the point is that time is never NaN in a pset.execute (see here), but it can be before the execute loop; e.g. during an initial sampling (particles.var = fieldset.var[particles]). That is why we need this error here and not in the execute loop

@VeckoTheGecko VeckoTheGecko Aug 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I see now

particle_positions = {"t": t, "z": z, "y": y, "x": x}
grid_positions = {}
grid_positions.update(_search_time_index(field, t))
Expand Down
19 changes: 19 additions & 0 deletions tests/test_particleset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,22 @@ def test_pset_default_z_closest_to_zero(depths):
pset = ParticleSet(fieldset, x=[0], y=[0])
expected_z = depths[np.argmin(np.abs(depths))]
assert np.isclose(pset.z[0], expected_z)


@pytest.mark.parametrize("npart", [1, 10])
@pytest.mark.parametrize("witht", [True, False])
def test_sampling_pset(fieldset, npart, witht):
# Test that inital value of a field gets sampled
fieldset.U.data[:] = 2.0

x = np.zeros(npart)
y = np.zeros(npart)
if witht:
t = npart * [np.timedelta64(0, "s")]
pset = ParticleSet(fieldset, x=x, y=y, t=t)
pset.sample, _ = fieldset.UV[pset]
np.testing.assert_allclose(pset.sample, 2.0, rtol=1e-12)
else:
with pytest.raises(ValueError, match="Time values for particles with indices .* cannot be NaN."):
pset = ParticleSet(fieldset, x=x, y=y)
pset.sample, _ = fieldset.UV[pset]