8000 Add support for overriding topography array ordering by mandli · Pull Request #647 · clawpack/geoclaw · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for overriding topography array ordering #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mandli
Copy link
Member
@mandli mandli commented May 17, 2025

This PR adds the abiility to override the priority that a topography file receives.

Previously topography files are ordered first by their resolution and then by the order they are given in setrun.py. This PR adds a parameter override_order so that the order that the topography files are specified in setrun.py always dictates the order that they will be used.

This is useful if you have two or more topography files that are nearly the same resolution and there is a preference for their order that does not match this.

@mandli mandli added the feature label May 17, 2025
@rjleveque
Copy link
Member

Thanks @mandli, this seems to work fine.

Notes:

To use this, set

    rundata.topo_data.override_order = True

in setrun.py, in which case the first topofile listed is given highest priority, with decreasing priority for each subsequent topofile. This means that the topo value in a grid cell is computed by first computing it using the last topofile listed, and then working backwards to update the value by removing the portion for the area of the cell covered by the previous topofiles and replacing it by the value computed on the intersection of the cell with that topofile.

Note that if

    rundata.topo_data.override_order = False

(the default value if you do not set it) then the original standard algorithm is used, with higher resolution topofiles (smaller area based on dxtopo*dytopo) having higher priority. If two topofiles have exactly the same resolution, then the one listed last has priority. Note that this is different from what might be expected based on the new override_order parameter!

@mandli
Copy link
Member Author
mandli commented May 25, 2025

Do we want to flip the order to the way that the usual algorithm does it? It would just require reversing the loop setting the order.

@mjberger
Copy link
Contributor
mjberger commented May 27, 2025 via email

@mandli
Copy link
Member Author
mandli commented May 27, 2025

It would make more sense to me to have the first file in the array be the top priority. I assume that this was originally an implementation detail and not intended as an interface. It may not be prudent to flip it still though.

@mandli
Copy link
Member Author
mandli commented Jun 11, 2025

So is there convergence around what the interface should be to this? I see three options:

  1. leave the implementation as is and make a note that the order are reversed,
  2. reverse the functionality here so that it matches the existing order of topography file priority, or
  3. reverse the interface to the existing interface, probably keep it internally the same, so that both again match.

I honestly think that I prefer option 3, but I see reasons for the others. We just need to make a decision before this becomes obsolete and needs to be reimplemented.

@mjberger
Copy link
Contributor
mjberger commented Jun 11, 2025 via email

@mandli
Copy link
Member Author
mandli commented Jun 11, 2025

I am not sure that this was ever really exposed, and this is just the interface. The internal representation would remain the same. Otherwise I like 2 better than 1 as well.

@rjleveque
Copy link
Member

I'm fine with #2 as well. In setrun.py we usually list the topofiles starting with the topo for the whole ocean and working down to the finer resolution DEMs, adding more at the end if we decide we need even finer resolution somewhere. Other people may do it differently, but this always seems natural to me. And coarsest to finest is the order that AMR grid levels are numbered too, so it would also be consistent with that.

@mandli
Copy link
Member Author
mandli commented Jun 11, 2025

@rjleveque that makes sense. I will implement the changes.

@mandli mandli force-pushed the add-topo-order-override branch from 2104b7a to 5d597b0 Compare June 12, 2025 13:56
@mandli
Copy link
Member Author
mandli commented Jun 12, 2025

This should now work as planned with the matching order of arrays (ordered from least to highest priority).

I also was thinking of putting in a test with the radial-ocean-island-fgmax and switching the correct order of the topography files. I originally thought that the island should not appear at all when doing this, but the island is included in both topography files, just at different resolutions. There is no test for that example so maybe this is a good one? Does not entirely cover what this was implemented for, but does cover the reversal in order.

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
0