Update queue selection to take walltime into account #1450
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| from CIME.XML.standard_module_setup import * | ||
| from CIME.utils import format_time | ||
| from CIME.XML.env_base import EnvBase | ||
| from CIME.utils import transform_vars, get_cime_root, convert_to_seconds | ||
| from copy import deepcopy | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
@@ -181,7 +181,7 @@ def make_batch_script(self, input_template, job, case, total_tasks, tasks_per_no | |
| fd.write(output_text) | ||
| os.chmod(job, os.stat(job).st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
| def set_job_defaults(self, batch_jobs, pesize=None, walltime=None, force_queue=None, allow_walltime_override=False): | ||
| if self.batchtype is None: | ||
| self.batchtype = self.get_batch_system_type() | ||
|
|
||
|
|
@@ -195,18 +195,35 @@ def set_job_defaults(self, batch_jobs, pesize=None, walltime=None, force_queue=N | |
| else: | ||
| task_count = int(task_count) | ||
|
|
||
| if force_queue: | ||
|
There was a problem hiding this comment. This is worth a close look. The behavior is basically the same as before expect that the algorithm will override a users requested walltime if the user walltime is causing us to fail to match any queues. There was a problem hiding this comment. In another issue (#1439), I was saying we should never override something explicitly set by the user. The lower walltime you fill in may not be long enough for what the user intended to do. They won't notice CIME submitted with a different walltime, the experiment will run and burn real time, and they they'll find it didn't run as long as they wanted/asked for. There was a problem hiding this comment. Yeah, that true. I'm wondering how to support the following case: A test that uses few procs but takes a long time, the test lives in acme_integration.
With the changes below, the walltime is automatically reduced and the test will at least have a chance to run. There was a problem hiding this comment. I could change the warning to something more conspicuous. |
||
| if not self.queue_meets_spec(force_queue, task_count, walltime=walltime, job=job): | ||
| logger.warning("WARNING: User-requested queue '%s' does not meet requirements for job '%s'" % (force_queue, job)) | ||
| else: | ||
| queue = self.select_best_queue(task_count, walltime=walltime, job=job) | ||
| if queue is None and walltime is not None: | ||
| # Try to see if walltime was the holdup | ||
| queue = self.select_best_queue(task_count, walltime=None, job=job) | ||
| if queue is not None: | ||
| # It was, override the walltime if a test, otherwise just warn the user | ||
| new_walltime = self.get_queue_specs(queue)[3] | ||
| expect(new_walltime is not None, "Should never make it here") | ||
| logger.warning("WARNING: Requested walltime '%s' could not be matched by any queue" % walltime) | ||
| if allow_walltime_override: | ||
| logger.warning(" Using walltime '%s' instead" % new_walltime) | ||
| walltime = new_walltime | ||
| else: | ||
| logger.warning(" Continuing with suspect walltime, batch submission may fail") | ||
|
|
||
| if queue is None: | ||
| logger.warning("WARNING: No queue on this system met the requirements for this job. Falling back to defaults") | ||
| default_queue_node = self.get_default_queue() | ||
| queue = default_queue_node.text | ||
| walltime = self.get_queue_specs(queue)[3] | ||
|
|
||
| walltime = self.get_queue_specs(queue)[3] if walltime is None else walltime | ||
| walltime = self._default_walltime if walltime is None else walltime # last-chance fallback | ||
|
|
||
| self.set_value("JOB_QUEUE", queue, subgroup=job) | ||
| self.set_value("JOB_WALLCLOCK_TIME", walltime, subgroup=job) | ||
| logger.debug("Job %s queue %s walltime %s" % (job, queue, walltime)) | ||
|
|
||
|
|
@@ -362,7 +379,6 @@ def _submit_single_job(self, case, job, depid=None, no_batch=False, batch_args=N | |
|
|
||
| function_name = job.replace(".", "_") | ||
| if not dry_run: | ||
| locals()[function_name](case) | ||
|
|
||
| return | ||
|
|
@@ -418,36 +434,64 @@ def get_job_id(self, output): | |
| jobid = search_match.group(1) | ||
| return jobid | ||
|
|
||
| def queue_meets_spec(self, queue, num_pes, walltime=None, job=None): | ||
|
There was a problem hiding this comment. Basically just an encapsulation of what select_best_queue was doing. |
||
| jobmin, jobmax, jobname, walltimemax, strict = self.get_queue_specs(queue) | ||
|
|
||
| # A job name match automatically meets spec | ||
| if job is not None and jobname is not None: | ||
| return jobname == job | ||
|
|
||
| if jobmin is not None and num_pes < int(jobmin): | ||
| return False | ||
|
|
||
| if jobmax is not None and num_pes > int(jobmax): | ||
| return False | ||
|
|
||
| if walltime is not None and walltimemax is not None and strict: | ||
|
There was a problem hiding this comment. The "strict" concept is implemented here. |
||
| walltime_s = convert_to_seconds(walltime) | ||
| walltimemax_s = convert_to_seconds(walltimemax) | ||
| if walltime_s > walltimemax_s: | ||
| return False | ||
|
|
||
| return True | ||
|
|
||
| def select_best_queue(self, num_pes, walltime=None, job=None): | ||
| # Make sure to check default queue first. | ||
| all_queues = [] | ||
| all_queues.append( self.get_default_queue()) | ||
| all_queues = all_queues + self.get_all_queues() | ||
| for queue in all_queues: | ||
| if queue is not None: | ||
| qname = queue.text | ||
| if self.queue_meets_spec(qname, num_pes, walltime=walltime, job=job): | ||
| return qname | ||
|
|
||
| return None | ||
|
|
||
| def get_queue_specs(self, queue): | ||
|
There was a problem hiding this comment. Just a generalization of getting properties of a queue. |
||
| """ | ||
| Get queue specifications by name. | ||
|
|
||
| Returns (jobmin, jobmax, jobname, walltimemax, is_strict) | ||
| """ | ||
| for queue_node in self.get_all_queues(): | ||
| if queue_node.text == queue: | ||
| jobmin = queue_node.get("jobmin") | ||
| jobmax = queue_node.get("jobmax") | ||
| jobname = queue_node.get("jobname") | ||
| walltimemax = queue_node.get("walltimemax") | ||
| strict = queue_node.get("strict") == "true" | ||
|
|
||
| return jobmin, jobmax, jobname, walltimemax, strict | ||
|
|
||
| expect(False, "Queue '%s' is unknown to this system" % queue) | ||
|
|
||
| def get_default_queue(self): | ||
| node = self.get_optional_node("queue", attributes={"default" : "true"}) | ||
| if node is None: | ||
| node = self.get_optional_node("queue") | ||
| expect(node is not None, "No queues found") | ||
|
There was a problem hiding this comment. I wasn't comfortable with a method called "get_default_queue" having an important side effect. |
||
| return node | ||
|
|
||
| def get_all_queues(self): | ||
| return self.get_nodes("queue") | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
"strict" is to deal with the fact that sometimes walltimemax is acts as a default and sometimes it's truly the longest walltime that a queue will accept.
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.
Wouldn't adding "walltimedefault" be a better option? I always read "walltimemax" as the maximum walltime that is allowed for that queue.
(I feel the "strict" flag somehow doesn't convey the intent)