Add prerequisite tags to all necessary test cases #560

xylar · 2020-05-14T08:54:46Z

These test cases depend on other test cases to run properly. Before this change, the only way to know that is via symlinks to directories outside of this test case. With this change, it will be easier for regression test suites to make sure prerequisites are present and that they always run before the test cases that depend on them (even if we run tests cases in parallel).

This PR should be tested in conjunction with #559 but can be merged independently of that PR without doing any harm. The new prerequisite tags simply will be ignored until the changes to manage_regression_suite.py make their way into ocean/develop.

xylar · 2020-05-14T09:01:13Z

@mark-petersen, see #559 (comment) for comments about testing this PR.

xylar · 2020-05-14T09:03:31Z

@pwolfram, you are welcome to test this out but mostly I'd appreciate you taking a look at the tags I added to test cases that you manage and make sure they seem correct. And, of course, that you're okay with these tags being added. It should be harmless to not include the tags if you would rather not, it just might mean you can't safely include your test cases in a regression test suite running in parallel, down the road when we hope to have that capability.

xylar · 2020-05-14T09:05:02Z

@jamilgafur and @vanroekel, this PR is less critical for you to have a look at, but you will eventually want the version of the QU240 test case with the prerequisite tags for the parallel version of the nightly regression test suite.

xylar · 2020-05-14T11:39:13Z

Testing

On Grizzly:

I ran the nightly regression test suite with this PR and manage_regression_suite.py from Add support for a prerequisite tag in COMPASS #559.
I ran the LIGHT regression suite and it had an error in the SOMA 32km test case related to a missing local file check_particle_sampling.py that I have previously mentioned (Fixes to COMPASS to support conda MPI #480 (comment)).
I discovered that the land-ice fluxes regression suite is broken, so I will fix that separately and won't try to test it here.
I ran the RPE test suite for 1 hour, and all but the last test completed successfully. The baroclinic_channel/1km test case started fine but timed out.

I didn't try to check bit-for-bit against the current ocean/develop, so @mark-petersen, that might be a good test with the nightly regression suite.

pwolfram · 2020-05-14T14:02:44Z

@xylar, is this the file you are looking for: https://github.com/MPAS-Dev/MPAS-Model/blob/ocean/coastal/testing_and_setup/compass/ocean/soma/analysis/check_particle_sampling.py? Sounds like the link was just broken?

pwolfram

Reviewed based on inspection and memory -- thanks @xylar !

pwolfram · 2020-05-14T14:11:10Z

testing_and_setup/compass/ocean/Gaussian_hump/USDEQU120cr10rr2/delaware/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="Gaussian_hump" resolution="USDEQU120cr10rr2" test="build_mesh"/>


pwolfram · 2020-05-14T14:11:25Z

testing_and_setup/compass/ocean/drying_slope/hybrid/1km/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="1km"/>


pwolfram · 2020-05-14T14:11:43Z

testing_and_setup/compass/ocean/drying_slope/hybrid/250m/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="250m"/>


pwolfram · 2020-05-14T14:11:50Z

testing_and_setup/compass/ocean/drying_slope/sigma/1km/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="1km"/>


pwolfram · 2020-05-14T14:12:01Z

testing_and_setup/compass/ocean/drying_slope/sigma/250m/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="250m"/>


pwolfram · 2020-05-14T14:12:09Z

testing_and_setup/compass/ocean/drying_slope/zstar/1km/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="1km"/>


pwolfram · 2020-05-14T14:12:16Z

testing_and_setup/compass/ocean/drying_slope/zstar/250m/config_driver.xml

@@ -1,4 +1,5 @@
 <driver_script name="run_test.py">
+	<prerequisite core="ocean" configuration="drying_slope" resolution="meshes" test="250m"/>


xylar · 2020-05-14T14:54:56Z

is this the file you are looking for: https://github.com/MPAS-Dev/MPAS-Model/blob/ocean/coastal/testing_and_setup/compass/ocean/soma/analysis/check_particle_sampling.py? Sounds like the link was just broken?

It looks likely. It seems like the test case isn't making the local link it should be. If you want that test case to get stress tested, it would be worth fixing.

pwolfram · 2020-05-14T14:56:04Z

@xylar, should this be bundled into some existing PR or its own PR?

xylar · 2020-05-14T14:58:14Z

@pwolfram, it's not related to any existing PRs I'm aware of so I think it should be its own.

vanroekel

Changes look good by visual inspection. Thanks for this addition @xylar

mark-petersen

Merged #559 and #560 locally, set up and ran nightly regression suite successfully. When I removed the init step:

+++ b/testing_and_setup/compass/ocean/regression_suites/nightly.xml
@@ -1,7 +1,4 @@
 <regression_suite name="nightly_ocean_test_suite">
-       <test name="Global Ocean 240km - Init Test" core="ocean" configuration="global_ocean" resolution="QU240" test="init">
-               <script name="run.py"/>
-       </test>

I get:

./manage_regression_suite.py -t ocean/regression_suites/nightly.xml ...
...
ValueError: Prerequisite of Global Ocean 240km - Performance Test does not precede it in the test suite: ocean global_ocean QU240 init

so everything works as expected. Looks good!

This can be merged after #559 is merged, and develop merged into ocean/develop. (We could merge it now and the tags are ignored, but that could be confusing)

xylar · 2020-05-19T06:49:35Z

@mark-petersen, thanks for the review. I agree, let's wait until #559 has made it to ocean/develop before merging this.

mark-petersen · 2020-05-19T11:42:10Z

Adding don't merge label until #559 has made it's way to ocean/develop.

xylar · 2020-08-20T12:47:28Z

@mark-petersen, I rebased this PR now that #559 and develop have been merged to ocean/develop. I also added the prerequisite tag to the WC14 test case. I believe this is ready to merge when you are accepting COMPASS merges to ocean/develop.

xylar · 2020-08-24T08:28:13Z

@sbrus89, I'm trying to get this work in following #485. It seems like there are tons of new test cases from ocean/coastal that need prerequisite tags. Could we have a conversation about this sometime soon? The hurricane tests are a particular sticking point because you currently link to the same config_driver.xml for many tests, but each of these has a different prerequisite.

These test cases depend on other test cases to run properly. Before this change, the only way to know that is via symlinks to directories outside of this test case. With this change, it will be easier for regression test suites to make sure prerequisites are present and that they always run before the test cases that depend on them (even if we run tests cases in parallel).

This required making individual copies of several shared `config_driver.xml` files

xylar · 2020-08-24T08:51:11Z

@sbrus89, and update: I went ahead and added prerequisite tags to all the hurricane tests, which required me to make copies of several config_driver.xml files. I hope that won't be a problem. Please take a look and make sure I didn't make any obvious mistakes. Currently, the prerequisite tags don't get used so there's not a great way to test if they are correct. We can test more extensively once we have parallel execution of the regression suite.

xylar · 2020-11-06T12:32:10Z

I believe we will go with a different approach to address this problem. It will be part of development of COMPASS v. 1.0 on the new COMPASS repo: https://github.com/MPAS-Dev/compass

xylar added Ocean COMPASS labels May 14, 2020

xylar requested review from mark-petersen, pwolfram and vanroekel May 14, 2020 08:54

xylar assigned mark-petersen May 14, 2020

xylar mentioned this pull request May 14, 2020

Add support for a prerequisite tag in COMPASS #559

Merged

xylar mentioned this pull request May 14, 2020

Fix land-ice fluxes regression suite #561

Merged

pwolfram approved these changes May 14, 2020

View reviewed changes

vanroekel approved these changes May 18, 2020

View reviewed changes

mark-petersen approved these changes May 18, 2020

View reviewed changes

mark-petersen added the Don't merge label May 19, 2020

This was referenced Jun 1, 2020

Add advection operator convergence test #583

Merged

COMPASS: rename e3sm_coupling and move it to be its own test #585

Closed

added parallel processing for regression suites #593

Closed

xylar removed the Don't merge label Aug 20, 2020

xylar force-pushed the ocean/add_prerequisite_tag branch 2 times, most recently from ed7fb16 to a9ef7b2 Compare August 20, 2020 12:46

xylar force-pushed the ocean/add_prerequisite_tag branch from a9ef7b2 to cbfe3f7 Compare August 20, 2020 14:39

xylar added 2 commits August 24, 2020 10:36

Add prerequisite tags to hurricane tests

1e90756

This required making individual copies of several shared `config_driver.xml` files

xylar force-pushed the ocean/add_prerequisite_tag branch from cbfe3f7 to 1e90756 Compare August 24, 2020 08:48

xylar requested a review from sbrus89 August 24, 2020 08:52

xylar added the in progress label Sep 8, 2020

xylar closed this Nov 6, 2020

xylar deleted the ocean/add_prerequisite_tag branch November 6, 2020 12:32

matthewhoffman added COMPASS Ocean labels Mar 17, 2021

		@@ -1,4 +1,5 @@
		<driver_script name="run_test.py">
		<prerequisite core="ocean" configuration="Gaussian_hump" resolution="USDEQU120cr10rr2" test="build_mesh"/>

Add prerequisite tags to all necessary test cases #560

Add prerequisite tags to all necessary test cases #560

Uh oh!

Conversation

xylar commented May 14, 2020

Uh oh!

xylar commented May 14, 2020

Uh oh!

xylar commented May 14, 2020

Uh oh!

xylar commented May 14, 2020

Uh oh!

xylar commented May 14, 2020

Testing

Uh oh!

pwolfram commented May 14, 2020

Uh oh!

pwolfram left a comment

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

pwolfram May 14, 2020

Choose a reason for hiding this comment

Uh oh!

xylar commented May 14, 2020

Uh oh!

pwolfram commented May 14, 2020

Uh oh!

xylar commented May 14, 2020

Uh oh!

vanroekel left a comment

Choose a reason for hiding this comment

Uh oh!

mark-petersen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xylar commented May 19, 2020

Uh oh!

mark-petersen commented May 19, 2020

Uh oh!

xylar commented Aug 20, 2020

Uh oh!

xylar commented Aug 24, 2020

Uh oh!

xylar commented Aug 24, 2020

Uh oh!

xylar commented Nov 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mark-petersen left a comment •

edited

Loading