Skip to content

Write 1D-compatible topo.data and dtopo.data for 1D runs - #745

Open
mandli wants to merge 7 commits into
clawpack:masterfrom
mandli:fix-1d-topo-data-format
Open

Write 1D-compatible topo.data and dtopo.data for 1D runs#745
mandli wants to merge 7 commits into
clawpack:masterfrom
mandli:fix-1d-topo-data-format

Conversation

@mandli

@mandli mandli commented Sep 7, 2026

Copy link
Copy Markdown
Member

Adds code to handle 1D versions of topo.data and dtopo.data. Note that the choice to not have the 1D code support the more complex 2D capabilities and therefore input formats was deliberate. If there should be more parity this PR should be changed to reflect that.

Eleven silent failures in crop_extent handling produced wrong answers with
no message: wrapped spellings gave empty grids, sub-cell and non-overlapping
crops gave the full file, a cropped type-4 read reported the full-file
extent, and buffer was dropped for every descriptor-cropped NetCDF file.
Adds the first Fortran regression coverage for the descriptor-crop path and
documents what a Topography represents across the antimeridian.

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5[1m]
A URL in topofiles was mangled by os.path.abspath into a bogus local path
before the reader's existing URL guard could see it; it is now rejected with
the fetch_remote_topo recipe. A crop crossing the antimeridian raised
'crop_bounds exceed file extent' even though _compute_lon_entries already
covered it; TopographyData.write now resolves entries before writing and
splits such a crop into one descriptor entry per side, carrying buffer and
coarsen onto each so the Fortran buffer fix applies. Adds byte-exact
topo.data goldens and the first end-to-end test of two entries with
differing lon_wrap_offset.

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5[1m]
GeoClaw's units policy was implemented in the NetCDF readers but written
down nowhere, so enforcement drifted. Adds dev/design/units_policy.md, a
UNITS_POLICY registry that both the doc table and a conformance test are
generated from, and fixes four violations: CSVFault.read parsed unit
annotations from column headings and discarded them (alaska1964.csv read as
Mw 5.20 instead of 8.53), input_units was a mutated mutable default that
silently declared SI, a unit-less dtopo time axis was silently assumed to be
seconds, and two docstrings claimed GeoClaw does not convert on read while
it does. Non-conforming rows are xfail(strict) so the remaining ASCII gaps
stay visible.

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5[1m]
Was not clear that the ASCII files have assumed units and needed
some clarification as to how the proposed unit rules apply there.
@mandli

mandli commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

PR Detailed Description -- Summarized by Claude

Companion: clawpack/clawutil#208, which passes num_dim in. Merge this
first
— it defaults to num_dim=2, so it is inert and harmless on its own,
whereas clawutil landing first against an older geoclaw would break every
geoclaw setrun. #208 guards that case anyway, but the ordering makes the guard
unnecessary.

What was broken

Every one of the six examples/1d_classic cases aborted at startup:

Reading data file: topo.data
At line 58 of file .../src/1d_classic/shallow/topo_module.f90 (unit = 7, file = 'topo.data')
Fortran runtime error: Bad logical value while reading item 1

1D and 2D share TopographyData/DTopoDataClawRunData builds the same
objects for both — but not the Fortran readers, and src/1d_classic was never
updated when #726 changed the file layouts. Line 58 is
read(iunit,*) override_topo_order, a logical: #726 dropped that line and
inserted the per-file preprocessing block, so 1D read a path string where it
expected T/F.

Fixing topo.data alone got five of six running. okada_dtopo then failed
further along:

At line 218 of file .../1d_classic/shallow/topo_module.f90 (unit = 7, file = 'dtopo.data')
Fortran runtime error: Bad integer for item 2 in list input

so dtopo.data needed the same treatment.

The two layouts are not symmetric

Worth spelling out, because the obvious symmetric fix is wrong in one direction
and unnecessary in the other.

read_topo_settings reads the path as a single list-directed item, so the
read is satisfied before it reaches any trailing comment. The topo path line
therefore keeps its # topo_path comment and works.

read_dtopo_settings reads two items in one statement:

read(iunit,*) dtopofname, dtopotype

List-directed input spans records, so the type sitting on the next line is
fine — that is why the pre-#726 format worked. What breaks it is the
# dtopo_path comment #726 added to the path line, which is consumed as
item 2. Hence the dtopo path line is written bare while the topo one is not.

There is a quieter consequence too. Had that read not aborted, the next
statement reads dt_max_dtopo, which would have landed on the preprocessing
block's crop_extent line and silently taken 0.0.

The change

TopographyData and DTopoData take num_dim (default 2, so 2D behaviour
and every existing direct construction are untouched) and write() branches on
it:

1D 2D
topo.data override_order line restored; path + type only unchanged 10-line block
dtopo.data bare path line; no preprocessing block unchanged
NetCDF descriptor omitted unchanged

Preprocessing attributes set on a 1D file now emit a UserWarning naming them
and pointing at topotools, rather than being dropped silently — the
deliberate non-parity in the description is a real decision, but a user who
sets coarsen in 1D should be told it will not happen.

Verification

check result
all six examples/1d_classic build and run pass
same, with the fix reverted Bad logical value while reading item 1, line 58 — the reported failure reproduced exactly
4 new unit tests in tests/test_data.py pass; all 4 fail without the fix
pytest tests/ -m "not remote" 519 passed, 4 skipped, 1 xfailed
examples/tsunami/chile2010 (2D, has a dtopo) 6 passed

The new tests pin both layouts directly — line counts, the override_order
line, the bare dtopo path line, absence of the preprocessing block — plus one
asserting the 2D layout is unchanged and that num_dim still defaults to 2.
That last one is the guard against a future edit quietly making 1D the default
shape.

Note for anyone re-running the examples: several 1d_classic cases need
python make_celledges.py (and make_dtopo.py for okada_dtopo) before
make .output, and a stale .data timestamp file will hide a regenerated
layout. Neither is related to this change.

@rjleveque

Copy link
Copy Markdown
Member

Thanks @mandli, this seems to work fine along with clawpack/clawutil#208

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5
Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5[1m]
@mandli
mandli force-pushed the fix-1d-topo-data-format branch from 31c3193 to db0d1ad Compare September 7, 2026 20:04
mandli added a commit to mandli/geoclaw that referenced this pull request Sep 7, 2026
…admap

Signed-off-by: Kyle Mandli <kyle.mandli@gmail.com>
Assisted-by: claude claude-opus-5[1m]
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.

2 participants