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

Add "--queue" as an option to cice.setup #143

Merged
merged 4 commits into from May 24, 2018

Conversation

mattdturner
Copy link
Contributor

This PR adds the ability to pass a batch-system queue to the cice.setup script.

  • Developer(s): Matt Turner

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? bfb

  • Is the documentation being updated with this PR? (Y/N) N
    If not, does the documentation need to be updated separately at a later time? (Y/N) Y
    Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
    which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

  • Default queues are defined in each individual env._

  • This should resolve an issue where some tests within the suites would require a larger walltime than the hard-coded queues.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

A few comments. First, I don't see how ICE_MACHINE_QUEUE becomes the default queue. There is this logic,

if (${acct} == ${spval}) then
      if (-e ~/.cice_proj) then
        set acct = `head -1 ~/.cice_proj`
      else
        set acct = ${ICE_MACHINE_ACCT}
      endif
endif

in cice.setup for acct but there is not an equivalent for queue that I can find. I would also maybe add support for having a ~/.cice_queue file as well, like acct/proj with same priority as acct if this needs to be fixed.

Second, it looks like we no longer have a queue dependence on runlength where short jobs might be debug and longer jobs might be in another queue. It can be done on a test by test basis, but will not happen in a test suite automatically where it could be handy. I think what's done in this implementation is OK, but we should also think about how we could support what's done now plus continue to support a runlength dependent queue. There are a couple ways this could be done, but nothing is entirely robust. I'm OK with what we have, but wonder if we could still keep that feature with the new --queue feature.

@mattdturner
Copy link
Contributor Author

Second, it looks like we no longer have a queue dependence on runlength where short jobs might be debug and longer jobs might be in another queue.

That was my original thought with having the queue passed via --queue to only be used for jobs that don't fit in the debug queues. That is a simple thing to do. However, having the queue be defined based on runlength when --queue is not passed would be a bit more difficult.

I forgot to set the default queue. I'll do that now.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

Thanks @mattdturner, I think this is OK. Lets give it a try and see what happens. I do worry about the change to a single queue for test suites, but I will merge this now and we can see how it goes.

@apcraig apcraig merged commit 1c20dfe into CICE-Consortium:master May 24, 2018
@mattdturner mattdturner deleted the batch_queue branch May 29, 2018 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants