Skip to content

Speed up for fillnan#511

Merged
bdestombe merged 14 commits intodevfrom
fillnan_do_uniform_grid
Sep 10, 2025
Merged

Speed up for fillnan#511
bdestombe merged 14 commits intodevfrom
fillnan_do_uniform_grid

Conversation

@bdestombe
Copy link
Collaborator

No description provided.

@bdestombe
Copy link
Collaborator Author

bdestombe commented Aug 13, 2025

#510

@bdestombe
Copy link
Collaborator Author

distance_transform_edt has the sampling argument that allows for rectangular cells instead of square cells

@bdestombe bdestombe changed the title Add uniform grid handling and fillnan functionality for structured grids Speed up for fillnan Aug 15, 2025
@bdestombe
Copy link
Collaborator Author

bdestombe commented Aug 15, 2025

Previously, the griddata interpolation function computed a value for all cells and not just for the cells with a missing value. This PR correctws that and thereby provides a great speed up for both structured grids and unstructured grids. The speedup for structured grids in which all cells have the same shape is even bigger.

@bdestombe bdestombe requested a review from OnnoEbbens August 16, 2025 09:33
Copy link
Collaborator

@OnnoEbbens OnnoEbbens left a comment

Choose a reason for hiding this comment

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

Thanks for speeding this up, this helps a lot. Most important comment is to use the nlmod/dims/shared.py methods for checking dimensions and gridtypes and maybe even add a function to check for rectangular cell shapes there.

I guess failing tests are not due to this PR right?

@bdestombe bdestombe requested a review from OnnoEbbens August 18, 2025 13:53
@bdestombe
Copy link
Collaborator Author

bdestombe commented Aug 18, 2025

The following allows to pass only neighboring cells with valid values for vdis grids:

_cell_connections = gwf.modelgrid.neighbors()
cell_connections = {k: _cell_connections[k] for k in np.where(ds.ahn.isnull())[0]}
unicons = set().union(*cell_connections.values()) - set(cell_connections.keys())

is_valid = np.zeros_like(ds["ahn"].values, dtype=bool)
is_valid[list(unicons)] = True

The following does the exact same for structured grids (as is implemented in this PR):

is_valid = binary_dilation(is_invalid) & ~is_invalid

Edit: proposed changes are now part of this PR

…vent memory sharing issues. Update tests for fillnan_da_vertex_grid to ensure proper handling of coordinates and validate results.
Copy link
Collaborator

@OnnoEbbens OnnoEbbens left a comment

Choose a reason for hiding this comment

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

Nice addition with the neighboring cells.

I was expecting the shared module to be more useful in this case but at least it was useful for one function. Just one last comment and then you can merge it (I already approved it).

If x is not provided and "x" is not in xar_in.coords, an error will be raised.
"""
if xar_in.dims != ("icell2d",):
if not is_vertex(xar_in):
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this will allow layered vertex arrays to pass which is not what you want (and avoided for the structured grid). I did not think of this when recommending the functions in the shared module.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @OnnoEbbens I don't follow you here. The code raises an error for everything that is not a vertex grid, which is desired here in this vertex function, right?

@github-project-automation github-project-automation bot moved this from Todo to In Progress in NHFLO Aug 19, 2025
@bdestombe bdestombe merged commit 7702c34 into dev Sep 10, 2025
2 of 4 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in NHFLO Sep 10, 2025
@bdestombe bdestombe deleted the fillnan_do_uniform_grid branch September 10, 2025 10:26
@bdestombe bdestombe restored the fillnan_do_uniform_grid branch September 17, 2025 12:53
@bdestombe bdestombe deleted the fillnan_do_uniform_grid branch September 17, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interpolating missing AHN values with nearest values

2 participants