Skip to content
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

SystemTestsCompareTwo multisubmit tries to do too much in phase 1 #1856

Closed
billsacks opened this issue Aug 31, 2017 · 0 comments · Fixed by #1857
Closed

SystemTestsCompareTwo multisubmit tries to do too much in phase 1 #1856

billsacks opened this issue Aug 31, 2017 · 0 comments · Fixed by #1857

Comments

@billsacks
Copy link
Member

billsacks commented Aug 31, 2017

In comparing #1830 with what made it to master, I noticed that the indentation of this block is wrong:

        # Compare results
        # Case1 is the "main" case, and we need to do the comparisons from there
        self._activate_case1()
        self._link_to_case2_output()

        self._component_compare_test(self._run_one_suffix, self._run_two_suffix, success_change=success_change)

-- this should be indented under the "Second run" conditional.

The current indentation leads the ERR test (and any other multi-submit test) to try to do component_compare_test after the first phase, leading to a FAIL result. This doesn't cause a test failure, because the FAIL is later overwritten with a PASS, but it is still incorrect.

I have a fix for this in an incoming PR.

@billsacks billsacks self-assigned this Aug 31, 2017
@ghost ghost assigned jedwards4b Aug 31, 2017
@ghost ghost added the in progress label Aug 31, 2017
jgfouca added a commit that referenced this issue Sep 1, 2017
Erp to restart tests

merge new erp test into new restart_tests abstract class, fix indentation in system_test_compare_two

Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes #1856

Update gh-pages html (Y/N)?:

Code review:
@ghost ghost removed the in progress label Sep 1, 2017
jgfouca pushed a commit that referenced this issue Nov 7, 2017
Update PEs for ne30 runs on Cetus/Mira:
* Update ne30-grid PEs on Mira/Cetus
  * faster default 128x16x4 PE layout with 0.88 sypd for ne30-wcycl case
* Reduce stacksize to 16M for 16 mpi-per-node to avoid OOM errors
* Relax XML schema for env-var attributes to enable matching to
    arbitrary case-vars with xmllint
* Switch ne120-atm compset from FC5AV1C-L to FC5AV1C-H01A

[BFB]
jgfouca pushed a commit that referenced this issue Feb 23, 2018
Update PEs for ne30 runs on Cetus/Mira:
* Update ne30-grid PEs on Mira/Cetus
  * faster default 128x16x4 PE layout with 0.88 sypd for ne30-wcycl case
* Reduce stacksize to 16M for 16 mpi-per-node to avoid OOM errors
* Relax XML schema for env-var attributes to enable matching to
    arbitrary case-vars with xmllint
* Switch ne120-atm compset from FC5AV1C-L to FC5AV1C-H01A

[BFB]
jgfouca pushed a commit that referenced this issue Mar 13, 2018
Update PEs for ne30 runs on Cetus/Mira:
* Update ne30-grid PEs on Mira/Cetus
  * faster default 128x16x4 PE layout with 0.88 sypd for ne30-wcycl case
* Reduce stacksize to 16M for 16 mpi-per-node to avoid OOM errors
* Relax XML schema for env-var attributes to enable matching to
    arbitrary case-vars with xmllint
* Switch ne120-atm compset from FC5AV1C-L to FC5AV1C-H01A

[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants