-
Notifications
You must be signed in to change notification settings - Fork 213
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
remove SMP_PRESENT and replace with BUILD_THREADED #4546
Conversation
system-testing expected to fail |
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.
Thanks, I think this is a good change.
@jgfouca, will there be a follow-up PR to fix things in E3SM?
|
@amametjanov you need to update mct/cime_config/config_component.xml |
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.
Checked in E3SM with SMS.f19_g16.X.pm-cpu_intel
and
azamat@perlmutter:login09:~/saul/E3SM
> git diff driver-mct/cime_config/config_component.xml
diff --git a/driver-mct/cime_config/config_component.xml b/driver-mct/cime_config/config_component.xml
index 7c8e83fa70..ea1de9ee8d 100644
--- a/driver-mct/cime_config/config_component.xml
+++ b/driver-mct/cime_config/config_component.xml
@@ -717,7 +717,7 @@
If TRUE, the component libraries are always built with OpenMP capability.</desc>
</entry>
- <entry id="SMP_PRESENT">
+ <entry id="BUILD_THREADED">
<type>logical</type>
<valid_values>TRUE,FALSE</valid_values>
<default_value>FALSE</default_value>
- Changing
NTHRDS_ATM
1->2->1 orFORCE_BUILD_SMP
F->T->F switchesBUILD_THREADED
F->T->F. - Need to do the rename during the merge to E3SM.
@amametjanov thank you for confirming. |
@jasonb5 do i need to fix something here or is the ball in your court? |
@jedwards4b It's in mine, I'll restart the tests shortly. |
@jedwards4b ccs_config needs to be updated in the CESM repo. |
@jasonb5 The file Externals.cfg that is used by docker needs to be updated. |
@jedwards4b I updated the container to use
|
I've been trying to figure that out.
|
Ah - the share should be share1.0.18 - we had it set to 17 |
The e3sm test is expected to fail because the mct driver config_compset.xml file needs to be changed to use BUILD_THREADED instead of SMP_PRESENT. |
It's removal in #4546 seems to have broken a number of less-common test types (ERR, MCC, IRT, PRE).
@@ -219,8 +219,6 @@ def create_clone( | |||
) | |||
) | |||
|
|||
newcase.case_setup() |
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.
Did this have anything to do with the SMP_PRESENT to BUILD_THREADED change?
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 think it was something I caught as being out of place during the development of that change, so it's not directly related to that but it was probably included in that PR. If you want to put it back and then open an issue to fix the problem by fixing the tests then I guess we can do it that way...
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 I'd like to do it that way.
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 think that we have a solution now that fixes the tests and leaves this out - are you okay with that?
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's removal in #4546 seems to have broken a number of less-common test types (ERR, MCC, IRT, PRE). MCC_P1.f19_g16_rx1.A ERROR: ERROR: must invoke case.setup script before calling build script ERR_Ln9.f45_g37_rx1.A Exception from case_test: [Errno 2] No such file or directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/archive/ERR_Ln9.f45_g37_rx1.A.anlgce_gnu.20240102_114925_1cza5r/rest' Exception from case_test: [Errno 20] Not a directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/ERR_Ln9.f45_g37_rx1.A.anlgce_gnu.20240102_114925_1cza5r/run/case2run/timing' IRT_N2_Vmct_Ln9.f19_g16_rx1.A Exception from case_test: [Errno 20] Not a directory: '/scratch/jenkins-slave/workspace/CIME_DEVELOPER_TESTS_MASTER/IRT_N2_Vmct_Ln9.f19_g16_rx1.A.anlgce_gnu.20240102_114925_1cza5r/run/case2run/timing' PRE.f19_f19.ADESP Exception from case_test: ERROR: Do not know about batch job case.test Test suite: the test cases above, by hand Test baseline: Test namelist changes: Test status: bit for bit Fixes [CIME Github issue #] User interface changes?: Update gh-pages html (Y/N)?:
SMP_PRESENT and BUILD_THREADED were both used in the same way and having two variables to do the same thing is confusing at best. This replaces all instances of SMP_PRESENT with BUILD_THREADED. Note that this may require changes in E3SM outside of CIME.
Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes #4544
User interface changes?: xml variable SMP_PRESENT is removed.
Update gh-pages html (Y/N)?: