-
Notifications
You must be signed in to change notification settings - Fork 180
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
Enable ESMF threading in the ufs-weather-model forecast #1371
Changes from 4 commits
0453d5d
d904cc9
0ade235
5c1e48b
9ba87a8
67b2760
435879b
27bc311
6c5ef9f
e6865cf
c796911
2a2b4a5
a2874b2
3690332
7b483c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ min_seaice="1.0e-6" | |
use_cice_alb=".true." | ||
|
||
# config.ufs # TODO: This is hard-wired for P8 and needs to be refactored. For now, use case C384 | ||
# TODO: Q. for @jessicameixner-noaa: can we make these defaults in config.ufs for C384? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To me, this is mostly for the commented out: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please follow: layout_x < layout_y. Also I am not sure how the WRTTASK_PER_GROUP_GFS=86 is coming out, it may be related to taking whole nodes, but it is really machine dependent, the number may not be good for wcoss2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @JessicaMeixner-NOAA
I am inclined to remove these as they are not wildly different than the ones in |
||
case "${CASE}" in | ||
"C384") | ||
DELTIM=300 | ||
|
@@ -27,10 +28,9 @@ case "${CASE}" in | |
#layout_y_gfs=16 | ||
#WRTTASK_PER_GROUP_GFS=86 | ||
WRTIOBUF="32M" # TODO: This value is for P8 w/ C384. Why not update/set this as default in config.fv3 C384? | ||
MEDPETS=300 # TODO: Use 300 instead of ATMPETS = layout_x * layout_y * 6 | ||
;; | ||
"C768") | ||
MEDPETS=300 | ||
*) # All other ${CASE} | ||
echo "config.defaults.s2sw: Using default settings for CASE=${CASE}" | ||
;; | ||
esac | ||
|
||
|
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.
Can you use a standard if/else here? IMO short-circuits are already harder to parse, and here you also have to know the operator precedence.
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.
Any reason to do it this way instead of making
nprocs
a local?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.
done on expansion.
And because this is not a function.
local
has no meaning outside a function (pretty sure about that, but not 100%)