Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Terrain-Aware Immersed Forcing Method #1115
Terrain-Aware Immersed Forcing Method #1115
Changes from 83 commits
d3087d5
69320fa
ec4086f
fc3724a
7797847
c3ad937
bd87c48
25ffb13
1b1b612
b735987
7fca03d
903ae8e
201602c
6055ba7
7e74c79
425d455
cd03664
5b401df
9dccb0d
88dfaf9
d871f53
db46f4a
b225703
383ce04
2e7d011
673d25f
7e8a3db
287d4b1
37abe4b
41beae0
b270562
702b762
6ba2347
2e4e780
c5549c3
25840f0
34416cc
45b6b3f
2e22d48
a9e2595
74b163e
890e8fe
8ac13e1
ad84719
63968e3
2473677
0e679d4
c7f463b
4ecbf15
02505aa
f81b41a
a481179
cd73d96
1693c6a
64c5d5c
7a54646
cd01d02
18ee504
13557f0
fbc496e
7da4b68
64c285b
ef6194b
e8e50eb
e005a1d
0254ceb
d1a5381
dea867a
406628d
f824119
bebab49
332dfef
f23d79a
ff98ab5
36e7fe7
19af0be
512e87f
03be705
87d2b87
62a97a9
532bcd4
af6b0ae
c686a84
5099d33
150adf0
06dd7a3
bef083c
1ed8544
cb3843b
f9b249f
a277b77
f64dce8
c1efb05
61630e2
c8fc54a
cd583e6
4c279fc
119a156
c274b0a
2161761
42d211c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto&
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a better way to find the first closest index to the reference height
x3
usingstd::lower_bound
andstd::upper_bound
or some such instead of this loop?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure that
std::lower_bound
is GPU safe. As manystd
functions are not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely agreed, forgot to update in the PR, but I clarified with @hgopalan offline to not use the
std
functions right after I had this comment here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to Michael's comments, is
drag
an integer/bool field? Also, perhaps call this something more descriptive thandrag
to not confuse this withm_drag
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the minimum value of z0 have some significance? Seems like it should be specified in a more obvious place or should be an input argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to have a -999 in the input file for water bodies sometimes. I can change it to a minimum roughness length variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these 1e-5 numbers seem kind of large to just avoid dividing by 0, which seems like what's happening here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it was added to avoid divide by zero. I can reduce it. Is there any default amex variable I can use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marchdf could you comment on a better way to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think the best practice is to set
const auto tiny = std::numeric_limits<amrex::Real>::epsilon()
before the loop and then use tiny throughout.