-
Notifications
You must be signed in to change notification settings - Fork 94
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
parentless sequential clock trigger spawns #5732
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -211,6 +211,11 @@ def __init__( | |||||
# Signatures of active functions (waiting on callback). | ||||||
self.active: list = [] | ||||||
|
||||||
# Gather named wall-clocks closet to current time. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
# (no need to check or spawn future clocks of tasks respectively). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you rephrase this comment? It doesn't seem to make much sense as worded!) |
||||||
self.wall_clock_labels: set = set() | ||||||
self.wall_clock_spawns: list = [] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this variable be named better? like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, will change.. maybe to broaden |
||||||
|
||||||
self.workflow_run_dir = workflow_run_dir | ||||||
|
||||||
# For function arg templating. | ||||||
|
@@ -316,6 +321,8 @@ def add_trig(self, label: str, fctx: SubFuncContext, fdir: str) -> None: | |||||
""" | ||||||
self.validate_xtrigger(label, fctx, fdir) | ||||||
self.functx_map[label] = fctx | ||||||
if fctx.func_name == "wall_clock": | ||||||
self.wall_clock_labels.add(label) | ||||||
|
||||||
def mutate_trig(self, label, kwargs): | ||||||
self.functx_map[label].func_kwargs.update(kwargs) | ||||||
|
@@ -380,7 +387,7 @@ def get_xtrig_ctx(self, itask: TaskProxy, label: str) -> SubFuncContext: | |||||
|
||||||
args = [] | ||||||
kwargs = {} | ||||||
if ctx.func_name == "wall_clock": | ||||||
if label in self.wall_clock_labels: | ||||||
if "trigger_time" in ctx.func_kwargs: | ||||||
# Internal (retry timer): trigger_time already set. | ||||||
kwargs["trigger_time"] = ctx.func_kwargs["trigger_time"] | ||||||
|
@@ -420,10 +427,12 @@ def call_xtriggers_async(self, itask: TaskProxy): | |||||
itask: task proxy to check. | ||||||
""" | ||||||
for label, sig, ctx, _ in self._get_xtrigs(itask, unsat_only=True): | ||||||
if sig.startswith("wall_clock"): | ||||||
if label in self.wall_clock_labels: | ||||||
# Special case: quick synchronous clock check. | ||||||
if wall_clock(*ctx.func_args, **ctx.func_kwargs): | ||||||
itask.state.xtriggers[label] = True | ||||||
if itask.tdef.is_parentless(itask.point): | ||||||
self.wall_clock_spawns.append(itask) | ||||||
self.sat_xtrig[sig] = {} | ||||||
self.data_store_mgr.delta_task_xtrigger(sig, True) | ||||||
LOG.info('xtrigger satisfied: %s = %s', label, sig) | ||||||
|
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.
Is this the right place to handle the spawning? The reason for the
if
block here is to "delete satisfied xtriggers no longer needed by any tasks" (from thextrigger_mgr.housekeep()
docstring).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.
I noticed that
housekeep_xtriggers
is satisfied if xtriggers are, so just piggy backed off that.. But could just use that spawns list.