-
Notifications
You must be signed in to change notification settings - Fork 94
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
base: master
Are you sure you want to change the base?
Conversation
Thanks @mandli, this seems to work fine. Notes: To use this, set
in Note that if
(the default value if you do not set it) then the original standard algorithm is used, with higher resolution topofiles (smaller area based on |
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. |
flipping it so that it's closer to the way it's usually specified makes sense.
… On May 25, 2025, at 4:48 PM, Kyle Mandli ***@***.***> wrote:
mandli
left a comment
(clawpack/geoclaw#647)
<#647 (comme
8000
nt)>
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.
—
Reply to this email directly, view it on GitHub <#647 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGUGCZXQE3NOBFZRLRANAD3AIUCDAVCNFSM6AAAAAB5KZDYECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMBYGA3TENBRHA>.
You are receiving this because you are subscribed to this thread.
|
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. |
So is there convergence around what the interface should be to this? I see three options:
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. |
I don't think you should change the previous functionality when you add a new feature.
So I prefer option 2.
— Marsha
… On Jun 11, 2025, at 8:45 AM, Kyle Mandli ***@***.***> wrote:
mandli
left a comment
(clawpack/geoclaw#647)
<#647 (comment)>
So is there convergence around what the interface should be to this? I see three options:
leave the implementation as is and make a note that the order are reversed,
reverse the functionality here so that it matches the existing order of topography file priority, or
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.
—
Reply to this email directly, view it on GitHub <#647 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGUGCZDZXEX5N4LMKEHR4T3DAQH5AVCNFSM6AAAAAB5KZDYECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNRSGU2TAMJYHA>.
You are receiving this because you commented.
|
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. |
I'm fine with #2 as well. In |
@rjleveque that makes sense. I will implement the changes. |
2104b7a
to
5d597b0
Compare
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 |
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 parameteroverride_order
so that the order that the topography files are specified insetrun.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.