Skip to content

Conversation

@simone-silvestri
Copy link
Collaborator

The choice of major basins is performed on main, on an extended grid that is used to compute the connectivity in case of a Periodic direction. It might be, therefore, that large basins such as the Caspian sea are repeated within the extended grid, and are removed from the extended grid without changing the interior of the bottom height which is what we actually want to mask out.

In this PR we still compute the connectivity (the labels) using an extended grid, but we rank the basins only on the labels in the interior of the grid, correcting the behavior of major_basins.
For example the output of this script:

grid = TripolarGrid(size = (360, 180, 1), z = (-5000, 0))
bottom_fields = [Array(interior(regrid_bathymetry(grid; minimum_depth = 10,  major_basins = i), :, :, 1)) for i in 1:6]
diffs = [bottom_fields[i] .- bottom_fields[i+1] for i in 1:5]

fig = Figure(size = (800, 1400), fontsize = 14)

for i in 1:5
    ax = Axis(fig[i, 1], title = "major_basins = $(i+1)$i", aspect = DataAspect())
    heatmap!(ax, diffs[i]; colorrange = (-0.001, 0.001))
    hidedecorations!(ax)
end

on main:
bathymetry_main

on this PR:
bathymetry_PR

cc @taimoorsohail

@simone-silvestri simone-silvestri marked this pull request as ready for review December 18, 2025 09:10
@glwagner
Copy link
Member

Did you know that the Caspian sea is 1000m deep and the remnant of an ancient ocean? Maybe you should include it in your bathymetry.

@navidcy navidcy added the bathymetry ⛰️ When things aren't as smooth as expected label Dec 18, 2025
@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.01%. Comparing base (41f27e7) to head (3aa29cd).

Files with missing lines Patch % Lines
src/Bathymetry.jl 0.00% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #708      +/-   ##
==========================================
- Coverage   41.06%   41.01%   -0.06%     
==========================================
  Files          57       57              
  Lines        3156     3160       +4     
==========================================
  Hits         1296     1296              
- Misses       1860     1864       +4     

☔ 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.

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

Labels

bathymetry ⛰️ When things aren't as smooth as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants