Support disk-level experiment purging in access-profiling#50
Support disk-level experiment purging in access-profiling#50minghangli-uni wants to merge 1 commit intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
===========================================
- Coverage 100.00% 99.37% -0.63%
===========================================
Files 16 16
Lines 791 797 +6
===========================================
+ Hits 791 792 +1
- Misses 0 5 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
micaeljtoliveira
left a comment
micaeljtoliveira
left a comment
There was a problem hiding this comment.
@minghangli-uni Thanks for creating this PR! I know this is still marked as draft, but I took the liberty of having a look and adding some suggestions. I hope that's fine.
| """Purges Payu experiments from the work directory. | ||
|
|
||
| Args: | ||
| branches (list[str] | None): List of branches to purge. If None, purges all existing branches. |
There was a problem hiding this comment.
I think we need to be very careful here in order to prevent users from making mistakes. In this case I think it would be best not to allow this parameter to be None and instead force the user to explicitly say they want all branches to be purged, either by saying something like branches="all" or by having some dedicated parameter (all_branches=True).
|
|
||
| runner = ExperimentRunner(runner_config) | ||
| runner.purge_experiments( | ||
| branches=branches, |
There was a problem hiding this comment.
Before passing the list of branches to the runner, I think one should check that all of them are linked to an experiment being managed by this instance of PayuManager.
| dry_run=dry_run, | ||
| remove_repo_dir=remove_repo_dir, | ||
| ) | ||
|
|
There was a problem hiding this comment.
After purging the branches, the corresponding experiment statuses need to be updated accordingly.
2 participants
closes #49