Skip to content

Speed up landice gridded_flood_fill with scipy.ndimage#948

Merged
matthewhoffman merged 2 commits intoMPAS-Dev:mainfrom
trhille:landice/speed_up_gridded_flood_fill
Apr 21, 2026
Merged

Speed up landice gridded_flood_fill with scipy.ndimage#948
matthewhoffman merged 2 commits intoMPAS-Dev:mainfrom
trhille:landice/speed_up_gridded_flood_fill

Conversation

@trhille
Copy link
Copy Markdown
Collaborator

@trhille trhille commented Mar 26, 2026

Speed up gridded_flood_fill with scipy.ndimage instead of nested loops. For the antarctica_8km_2024_01_29.nc, this reduced the time from about 7s to 0.005s. This could make it much more manageable to use high-resolution gridded data sets to determine cell spacing for large meshes.

Checklist

  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

@trhille
Copy link
Copy Markdown
Collaborator Author

trhille commented Mar 26, 2026

Testing

When using the antarctica_1km_2024_01_29.nc source file, the current implementation of landice.mesh.gridded_flood_fill in main takes 495.47 seconds. With these changes, it takes 0.32 seconds.
@xylar @matthewhoffman, I think you'll like this :)

I still need to confirm that these result in similar masks.

@matthewhoffman matthewhoffman added land ice in progress This PR is not ready for review or merging labels Mar 29, 2026
@matthewhoffman matthewhoffman marked this pull request as draft March 29, 2026 16:45
trhille added 2 commits April 20, 2026 09:20
Speed up gridded_flood_fill with scipy.ndimage instead of nested
loops. For the antarctica_8km_2024_01_29.nc, this reduced the time from
about 7s to 0.005s. This could make it much more manageable to use
high-resolution gridded data sets to determine cell spacing for
large meshes.
@trhille trhille force-pushed the landice/speed_up_gridded_flood_fill branch from 20fa45e to bfdd243 Compare April 20, 2026 16:20
@trhille
Copy link
Copy Markdown
Collaborator Author

trhille commented Apr 20, 2026

Here are results for the 3–30km GIS mesh, using the head of main (left) and the head of this branch (right). Results Both meshes have nCells = 54173 and areaCell fields are identical between the two. Using this branch, gridded_flood_fill took 0.0086 seconds; using main it took 7.55 s.

image

@trhille trhille requested a review from matthewhoffman April 20, 2026 16:55
@trhille trhille removed in progress This PR is not ready for review or merging labels Apr 20, 2026
@trhille trhille marked this pull request as ready for review April 20, 2026 16:56
Copy link
Copy Markdown
Member

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@trhille , this looks great, and thanks for adding the test showing no answers change. CI is failing for some unrelated reason, so once I sort that out, I will merge.

@matthewhoffman
Copy link
Copy Markdown
Member

@xylar , it looks like the CI is failing when trying to build jigsaw. I forced the CI to rerun and got the same error. I'm confused because this branch is based on main, and the CI appeared to pass on the most recent PR. I can dig deeper, but wanted to check if you had any insight into this, as I know it happens periodically.

@xylar
Copy link
Copy Markdown
Collaborator

xylar commented Apr 21, 2026

The way of building jigsaw changes (or can change) in #944. I don't really want to debug the "old" way of building jigsaw.

@matthewhoffman matthewhoffman merged commit 174dd59 into MPAS-Dev:main Apr 21, 2026
2 of 11 checks passed
@matthewhoffman
Copy link
Copy Markdown
Member

Based on group discussion, merging without CI passing.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants