Skip to content

Update Bathymetry.jl#702

Open
ruoyan88 wants to merge 6 commits intoCliMA:mainfrom
ruoyan88:fix-bathymetry
Open

Update Bathymetry.jl#702
ruoyan88 wants to merge 6 commits intoCliMA:mainfrom
ruoyan88:fix-bathymetry

Conversation

@ruoyan88
Copy link
Copy Markdown

Problem

regrid_bathymetry fails when used with RotatedLatitudeLongitudeGrid due to a strict grid type check in interpolate_bathymetry_in_passes.

Solution

Remove the unnecessary grid type validation as suggested in the discussion. The interpolate! function only depends on the source grid type, not the destination grid type.

Changes

  • Replace the strict type check with string(nameof(typeof(target_grid))) to support any grid type while preserving logging functionality.

Closes #666

Fix regrid_bathymetry for other types of grid
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.03%. Comparing base (08ccc76) to head (8ca072d).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/Bathymetry.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #702       +/-   ##
===========================================
+ Coverage   11.46%   32.03%   +20.56%     
===========================================
  Files          59       59               
  Lines        3261     3318       +57     
===========================================
+ Hits          374     1063      +689     
+ Misses       2887     2255      -632     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@glwagner glwagner left a comment

Choose a reason for hiding this comment

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

We want to keep the error, and extend support to any OrthogonalSphericalShellGrid probably.

@ruoyan88
Copy link
Copy Markdown
Author

@glwagner Thanks for the review!
So you would prefer to keep the error as-is and extend it to cover OrthogonalSphericalShellGrid (and potentially other orthogonal grids in the future)?
Should I revert my current changes and implement the stricter check instead?

@glwagner
Copy link
Copy Markdown
Member

hmm actually I think I misunderstood the code. This is purely getting the name of the grid...

I take it back, this is ok to merge. However, I would move where we define gridtype and put it next to the string, so that it is more obvious the purpose of gridtype

Copy link
Copy Markdown
Member

@glwagner glwagner left a comment

Choose a reason for hiding this comment

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

I suggest moving gridtype = string(nameof(typeof(target_grid))) to line 229.

@ruoyan88
Copy link
Copy Markdown
Author

@glwagner Thanks a lot for reviewing and approving! 🚀

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.

regrid_bathymetry does not support RotatedLatitudeLongitudeGrid

3 participants