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

Give users more info and control over key run commands #1359

Merged
merged 8 commits into from
Apr 27, 2017

Conversation

jgfouca
Copy link
Contributor

@jgfouca jgfouca commented Apr 14, 2017

Adds new tool 'preview_run' to give users a preview of what their batch submissions and mpirun will look like.

Adds new override XML variables so users can exercise direct control over there batch and mpirun cmds if they want (advanced users only).

Test suite: scripts_regression_tests --fast
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #1263

User interface changes?: Signficant: new preview_run tool and new XML variables

Code review: Many

Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand how these override commands work - are they set in config_machines.xml for a particular system or in an individual case?

@@ -299,16 +299,23 @@ def submit_jobs(self, case, no_batch=False, job=None, batch_args=None):
if prereq is None or job == firstjob:
prereq = True
else:
if dry_run:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just add dry_run to the condition on line 299?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dry_run is the only case where we want to fake-out the BUILD_COMPLETE prereq.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see what you mean. We want to preserve the other prereqs besides the BUILD_COMPLETE one so that we get an accurate list of which jobs will be run.

@jgfouca
Copy link
Contributor Author

jgfouca commented Apr 17, 2017

@jedwards4b ever since the change to python, users have been asking for direct control over the submit and mpirun commands. Apparently, it was part of their workflow to open the shell scripts generated by CIME2 and modify these commands directly. The override XML values allow them to do this again.

@jgfouca
Copy link
Contributor Author

jgfouca commented Apr 20, 2017

@mvertens @rljacob still waiting for reviews for this PR as it represents a significant user interface change.

@jedwards4b
Copy link
Contributor

@rljacob Still waiting for your action on this one...

@jedwards4b
Copy link
Contributor

@rljacob What is the hold up on this one?

@rljacob
Copy link
Member

rljacob commented Apr 24, 2017

Finding time to try it out. Doing it today.

@rljacob
Copy link
Member

rljacob commented Apr 25, 2017

I tried building an X case with this branch on blues "./create_newcase -case Xcase2 -compset X -res f19_g16" and the "preview_run" command was not in my caseroot even after running case.setup and case.build.

@@ -2602,6 +2602,15 @@
<desc>The machine wallclock setting. Default determined in config_machines.xml can be overwritten by testing</desc>
</entry>

<entry id="BATCH_COMMAND">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename to BATCH_COMMAND_FLAGS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@rljacob
Copy link
Member

rljacob commented Apr 26, 2017

Make sure preview_run shows up in caseroot so people see it. Like preview_namelist.

@jgfouca
Copy link
Contributor Author

jgfouca commented Apr 27, 2017

Make sure preview_run shows up in caseroot so people see it. Like preview_namelist.

Done

jgfouca added 3 commits April 27, 2017 13:44
…ools

* origin/master: (110 commits)
  Fixed Some Tabulation
  Removed Tabs from create_newcase
  Change PET test to fit on desktops
  remove broken pfunit directory
  nag needs a lib pointer
  TOTAL_CORES needs to be set before xml files are locked
  Fix mistake
  PIO settings need to happen before init_derived_attributes
  Order of operations was not correct
  Re-initialize key case values upon case.setup
  Double nodes instead of halving tasks
  PET must halve TASKS when doubling THRDS
  Fix big mistake in scripts_regression_tests
  handle case when executable is none (scripts_regression_tests)
  Fix minor bug in test_scheduler
  Minor changes to xmlchange
  Print time built per model
  Add input-dir to create_newcase
  More changes for testreport from code review.
  Add testreporter.template.  Fix comment in test_reporter.py
  ...
@rljacob
Copy link
Member

rljacob commented Apr 27, 2017

blogin3[98]: ./preview_run
BATCH SUBMIT:
    case.run -> qsub    -q shared -l walltime=01:00:00 -A ACME case.run 

MPIRUN: mpiexec  -n 16  /lcrc/project/ACME/jacob/acme_scratch/Xcase2/bld/acme.exe  >> acme.log.$LID 2>&1 

Awesome.

@jgfouca jgfouca merged commit dc29942 into master Apr 27, 2017
@jgfouca jgfouca deleted the jgfouca/new_query_tools branch April 27, 2017 20:47
jgfouca pushed a commit that referenced this pull request Jun 2, 2017
* Use proper syntax to refer to OMP_NUM_THREADS in config_machines.xml.
    Must use ENV syntax.
* Move setting of OMP_NUM_THREADS earlier so it will be captured
    by provenance saving.
* Avoid hyper-threading on Edison.

Fixes #1355

[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 this pull request may close these issues.

4 participants