Skip to content

Implement add_rose_experiment and delete/archive/run_experiments to CylcRoseManager#59

Merged
micaeljtoliveira merged 7 commits into
ACCESS-NRI:mainfrom
edoyango:cycl-rose-manager
Jun 18, 2026
Merged

Implement add_rose_experiment and delete/archive/run_experiments to CylcRoseManager#59
micaeljtoliveira merged 7 commits into
ACCESS-NRI:mainfrom
edoyango:cycl-rose-manager

Conversation

@edoyango

Copy link
Copy Markdown
Collaborator

This PR implements features in CylcRoseManager that are missing (compared to PayuManager).

run_experiments uses the rose suite-run cmd as I don't think there's a good python api to use for rose. rose suite-run is executed with --no-gcontrol so the gui isn't launched. Like in PayuManager, the experiments' status is set to DONE immediately after launching the pipeline.

@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7325277) to head (726d47d).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #59   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          948      1015   +67     
=========================================
+ Hits           948      1015   +67     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/access/profiling/cylc_manager.py
Comment thread src/access/profiling/cylc_manager.py Outdated
Comment thread src/access/profiling/cylc_manager.py Outdated
@micaeljtoliveira

Copy link
Copy Markdown
Member

@edoyango Forgot to say in the review that you need to rebase on main to get the Conda build CI failure fixed.

@edoyango edoyango force-pushed the cycl-rose-manager branch from fec1a39 to 4f4890a Compare May 12, 2026 22:09
@edoyango edoyango force-pushed the cycl-rose-manager branch from 602a727 to 1dc48ef Compare June 17, 2026 23:03

@micaeljtoliveira micaeljtoliveira left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@edoyango Thanks for the changes! I've got another couple of comments. Almost there ;)

Comment thread src/access/profiling/manager.py Outdated
Comment thread src/access/profiling/manager.py Outdated
@edoyango edoyango force-pushed the cycl-rose-manager branch from 1dc48ef to bb8e5b6 Compare June 18, 2026 02:02
@micaeljtoliveira

Copy link
Copy Markdown
Member

@edoyango You need to sign your commits to unblock the merge.

edoyango and others added 7 commits June 18, 2026 12:12
…to CylcRoseManager

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both PayuManager and CylcRoseManager had identical logic for validating
and resolving experiment name selections. Move it to a single base-class
helper so each subclass only owns its deletion mechanics.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the Gadi-specific /scratch/<project>/<user>/cylc-run/<rose> run
path construction (and PROJECT env lookup) with an explicit run_path
argument, keeping CylcRoseManager general across sites.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the shared selection, validation and manager-state bookkeeping into
a concrete ProfilingManager.delete_experiments, which delegates the
per-experiment on-disk deletion to a deferred _delete_experiment method
implemented by each concrete manager. This inverts the dependency so
control flows from the abstract class to its children, replacing the
_resolve_names helper.

CylcRoseManager._delete_experiment removes the experiment and run
directories. PayuManager keeps a thin delete_experiments override to
accept remove_repo_dir and implements _delete_experiment via the
experiment runner per branch; its public selection arguments are renamed
branches/all_branches -> experiments/all_experiments for consistency.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Micael Oliveira <micael.oliveira@anu.edu.au>
@edoyango edoyango force-pushed the cycl-rose-manager branch from 67c2cd7 to 726d47d Compare June 18, 2026 02:14
@edoyango

Copy link
Copy Markdown
Collaborator Author

@edoyango You need to sign your commits to unblock the merge.

done!

I forgot to setup signing on my new machine.

@micaeljtoliveira micaeljtoliveira merged commit fa7faff into ACCESS-NRI:main Jun 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants