-
Notifications
You must be signed in to change notification settings - Fork 216
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
compare two test case names match #1832
Conversation
@jedwards4b is this guaranteed to work on all systems? I'm thinking of machines that put their builds and runs on separate file systems. |
Yes I think so since both the caseroot and cime_output_root are handled explicitly. Do you have an example of a machine that does this? |
I can test this by adding the --test-root option to create_test, trying it now. |
@jedwards4b I pushed a commit with a minor grammar fix. Now reviewing for more substantive things. |
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 happy with the general idea of this PR. But I have the same comment that I did for #1830 : I find it confusing to have a directory structure where (for example) /glade/scratch/sacks/ERP.f09_g16.X.cheyenne_intel.20170821_122057_wklm6k
is the base case, and /glade/scratch/sacks/ERP.f09_g16.X.cheyenne_intel.20170821_122057_wklm6k/ERP.f09_g16.X.cheyenne_intel.20170821_122057_wklm6k
is the restart case... and the same for CIME_OUTPUT_ROOT: There is no indication from the directory structure that the latter is the restart case, and instead you see the confusing situation where the case directory has a subdirectory with the same name.
I feel it would be more clear if another level were added to the directory structure, so we had something like /glade/scratch/sacks/ERP.f09_g16.X.cheyenne_intel.20170821_122057_wklm6k/rest_case/ERP.f09_g16.X.cheyenne_intel.20170821_122057_wklm6k
- see also the example in #1647 (comment)
@@ -92,7 +92,10 @@ def __init__(self, | |||
self._case2 = None | |||
|
|||
self._setup_cases_if_not_yet_done() | |||
|
|||
# Since case 2 has the same name as case1 its CIME_OUTPUT_ROOT must also be different | |||
self._case2.set_value("CIME_OUTPUT_ROOT", |
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.
This should go in the _setup_cases function, in order to keep the init method cleaner and so that any problem is protected by the try... except block in _setup_cases_if_not_yet_done.
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.
moved
Now if the case1 name is "thiscase" case2 will be located in thiscase/case2/thiscase and the |
Okay, I'm happy with this now - thanks! |
@jedwards4b should I merge? |
@jgfouca yes |
Remove machine-specific flags and avoid FP exceptions on invalid FP values with GNU. Addresses #1832 [BFB]
This PR makes the case2 casename in system_tests_compare_two match the case one casename and nests both the case directory and the output directories inside that of case1. This is a precursor for a PR to address issue #1640
Addresses #1640
Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes
User interface changes?:
Update gh-pages html (Y/N)?: N
Code review: