-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: user error for hetpar start and end core #1262
base: release
Are you sure you want to change the base?
Conversation
Hey @pgierz! I might be wrong, but I think |
whoops, yeah...I'll take care of that, sorry. |
@@ -220,6 +250,7 @@ def calculate_requirements(config, cluster=None): | |||
) | |||
|
|||
else: | |||
# FIXME(PG): ...what? Just continue??? |
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, think about oasis
or recom
for fesom2: they are components for ESM-Tools, but they are libraries, meaning they don't need cores themselves.
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.
Alright. Then I think we should write something to that effect:
# FIXME(PG): ...what? Just continue??? | |
# NOTE: For components like oasis or recom, no "own" cores are needed |
start_core = config[model]["start_core"] | ||
end_core = config[model]["end_core"] | ||
|
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'm almost certain that removing this will kill the het_par_wrappers functionality.
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.
One would think so. However, the start_core
and end_core
variables are not used anywhere in that function. Note that other functions further down might need those keys, but they aren't needed here specifically. To avoid getting errors in the wrong place, I removed them.
No description provided.