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

Add check for UUIDs when checking for existence of archives #425

Merged

Conversation

jo-basevi
Copy link
Collaborator

@jo-basevi jo-basevi commented Mar 15, 2024

In #420, there’s a bug with using branching logic, where an existing experiment is not detected if its archive does not exist. So if an experiment was created then deleted on a branch in a control directory that has a corresponding legacy archive, subsequent payu setup/run calls, it’ll use the legacy archive.

This PR so far adds an additional check when checking for the existence of archives, also check for metadata with a matching UUID.

This isn’t a perfect check as it requires payu setup/payu run/ payu checkout to be run on the legacy experiment which will then subsequently create a UUID and metadata in the legacy archive if it doesn’t already exist. Otherwise, if there is no metadata associated with the legacy archive and a new UUID has not been generated, it’s difficult to know whether to use a legacy archive or not.

Closes #420

TODO:
- Write up a short explainer for "recommendations for use with legacy experiments"

@jo-basevi
Copy link
Collaborator Author

I've simplified the logic in the check to only fail when there exists a UUID in the archive metadata but it is not equal to the UUID in the control directory metadata.

So it'll still work for legacy archives with no UUIDs or metadata. So the recommendation when running a legacy experiment but want to run a related experiment on a different branch in the same control directory is to firstly run: payu setup on the legacy experiment branch to ensure a UUID is generated, and metadata is copied across to the legacy archive.

@jo-basevi jo-basevi marked this pull request as ready for review April 3, 2024 00:48
@aidanheerdegen
Copy link
Collaborator

Doesn't this interact rather heavily with #427? I'm seeing code I think was modified in the other PR.

@jo-basevi
Copy link
Collaborator Author

Yes, I'll need rebase the changes onto main and then run a couple tests

@jo-basevi jo-basevi marked this pull request as draft April 4, 2024 22:00
@jo-basevi jo-basevi force-pushed the 420-existing-legacy-archives branch from 7ac0f34 to 00964dd Compare April 5, 2024 02:15
@jo-basevi jo-basevi marked this pull request as ready for review April 5, 2024 02:22
Copy link
Collaborator

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

LGTM. All much cleaner and easier to follow.

@aidanheerdegen aidanheerdegen merged commit 9bd3ba6 into payu-org:master Apr 5, 2024
7 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.

Existing experiment is not detected if archive does not exist
2 participants