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

Re-evaluate the --initial/final/start/stop-cycle-point options for restarts #4062

Closed
5 tasks done
MetRonnie opened this issue Feb 4, 2021 · 6 comments · Fixed by #4313 or #4543
Closed
5 tasks done

Re-evaluate the --initial/final/start/stop-cycle-point options for restarts #4062

MetRonnie opened this issue Feb 4, 2021 · 6 comments · Fixed by #4313 or #4543
Assignees
Labels
could be better Not exactly a bug, but not ideal.
Milestone

Comments

@MetRonnie
Copy link
Member

MetRonnie commented Feb 4, 2021

The ignore value for the cycle point cli options (--icp=ignore, --startcp=ignore etc) for cylc play are potentially confusing, and the behaviour is a little inconsistent.

Final cycle point

When the scheduler shuts down, the final cycle point is saved to the database (edit: only if the CLI opt --fcp was used to set it, not if it was only specified in flow.cylc). On restart, this DB value is loaded.

--fcp=ignore causes Cylc to ignore the value stored in the DB, and instead use the value in flow.cylc[scheduling]final cycle point. The two might be different:

  • If the cli option --fcp=<point> was used in a previous run to override flow.cylc (then the cli-supplied value would have be saved in the DB).

Issues

These apply to all 4 types of cycle point:

  • The name ignore is potentially misleading, as one might get the idea it will basically set the final cycle point to None. Maybe --*-cycle-point=reload/refresh/reset is clearer? However, the cycle point is reloaded from flow.cylc[scheduling] anyway if not previously set in the cli*. (Resolved in Change 'ignore' to 'reload' for cycle point cli opts #4313)

Initial cycle point

Initial cycle point is handled in much the same way as final cycle point. However, this time there is a second way the value stored in the DB might be different from flow.cylc[scheduling]:

  • If flow.cylc[scheduling]initial cycle point = now or next() or previous(), in which case the timepoint is evaluated at the time of first start and stored in the DB

Issues

  • In a restart, you can't change the initial cycle point using the cli opt. Any value for --icp=<point> apart from ignore will raise an error. However, you could in fact change the initial cycle point in flow.cylc[scheduling]. So why not using the cli opt? (Resolved: --icp option not valid for restart since Change 'ignore' to 'reload' for cycle point cli opts #4313)

Start cycle point

Similar to above, but there is no way of specifying the start cycle point in flow.cylc.

Issues

Stop cycle point

Issues


*Apart from stopcp which is only reloaded from flow.cylc if not stored in the DB (which is the case if it was not set, or it was set but was reached)

@MetRonnie MetRonnie mentioned this issue Feb 4, 2021
7 tasks
@MetRonnie MetRonnie added could be better Not exactly a bug, but not ideal. question Flag this as a question for the next Cylc project meeting. labels Feb 5, 2021
@MetRonnie MetRonnie added this to the cylc-8.0.0 milestone Feb 5, 2021
@MetRonnie MetRonnie changed the title Reconsider the --ignore-initial/final/start/stop-cycle-point options for cylc restart Re-evaluate the --ignore-initial/final/start/stop-cycle-point options for cylc restart Feb 5, 2021
@MetRonnie MetRonnie changed the title Re-evaluate the --ignore-initial/final/start/stop-cycle-point options for cylc restart Re-evaluate the --initial/final/start/stop-cycle-point options for restarts Jul 20, 2021
@MetRonnie
Copy link
Member Author

MetRonnie commented Jul 20, 2021

In a restart, you can't change the initial cycle point using the cli opt. Any value for --icp=<point> apart from ignore will raise an error. However, you could in fact change the initial cycle point in flow.cylc[scheduling]. So why not using the cli opt?

Coming back to this, I'm pretty sure --icp=ignore doesn't do anything and will raise a point parsing error

@MetRonnie MetRonnie linked a pull request Aug 24, 2021 that will close this issue
7 tasks
@wxtim wxtim self-assigned this Oct 19, 2021
@wxtim wxtim removed their assignment Nov 2, 2021
@MetRonnie
Copy link
Member Author

The 2 questions that I think we should come to a decision on for 8.0rc1:

In a restart, you can't change the initial cycle point using the cli opt. Any value for --icp=<point> apart from ignore will raise an error. However, you could in fact change the initial cycle point in flow.cylc[scheduling]. So why not using the cli opt?

--stopcp=ignore is inconsistent with the others. While it ignores the stop cycle point in the database, it doesn't use [scheduling]stop after cycle point, it instead uses the final cycle point (from the database if set there, otherwise flow.cylc). I think it should be changed to use [scheduling]stop after cycle point if set, otherwise the final cycle point.

@hjoliver
Copy link
Member

I'm struggling to remember exactly why we have these as CLI options. Even setting the ICP on the command line, as opposed to changing it in the config file, is a pretty minor convenience with a significant downside: the installed config file doesn't reflect what actually got run. Maybe the need for it is less now that we have proper workflow installation built in??

@oliver-sanders
Copy link
Member

I think this is low priority and can be bumped to 8.x?

@MetRonnie
Copy link
Member Author

Proposal (need to decide whether to implement this for 8.0rc1):

  • Change --stopcp=reload option to use [scheduling]stop after cycle point (falling back to the final cycle point) instead of just using the final cycle point outright (this current behaviour makes even less sense now that ignore has been changed to reload)

As for the initial cycle point bit, I have checked, and in #4313 I made the --icp option entirely illegal for restarts. I think this is enough to tick off that part of the issue?

@MetRonnie
Copy link
Member Author

All points addressed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment