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 git hash to metadata output #888

Merged
merged 5 commits into from
Feb 11, 2025
Merged

Conversation

BenWibking
Copy link
Collaborator

Description

This adds the git commit hash to metadata.yaml for plotfiles and checkpoints. This allows users to track the exact provenance of simulation outputs.

Related issues

Closes #852.

Checklist

Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an x inside the square brackets [ ] in the Markdown source below:

  • I have added a description (see above).
  • I have added a link to any related issues (if applicable; see above).
  • I have read the Contributing Guide.
  • I have added tests for any new physics that this PR adds to the code.
  • (For quokka-astro org members) I have manually triggered the GPU tests with the magic comment /azp run.

@BenWibking BenWibking marked this pull request as ready for review February 10, 2025 05:57
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 10, 2025
@BenWibking
Copy link
Collaborator Author

/azp run

@dosubot dosubot bot added the enhancement New feature or request label Feb 10, 2025
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@BenWibking BenWibking enabled auto-merge February 10, 2025 07:20
@markkrumholz
Copy link
Collaborator

Changes generally look fine, but I have a question about how you expect the warning about untracked changes to work for people outside our research group, who do not have write access to our repo, but who will presumably want to create their own problem generators. Is the idea that they will create their own forks of the repo into which to check their problem setups, and the track changes warning will then be checked against their fork? Is the expectation that they should submit their problem setups to the main repo?

@BenWibking
Copy link
Collaborator Author

Changes generally look fine, but I have a question about how you expect the warning about untracked changes to work for people outside our research group, who do not have write access to our repo, but who will presumably want to create their own problem generators. Is the idea that they will create their own forks of the repo into which to check their problem setups, and the track changes warning will then be checked against their fork? Is the expectation that they should submit their problem setups to the main repo?

I think we can be agnostic as to what branch the changes are committed to. As long as the changes have been committed (even only locally), then the resulting git hash can be used to uniquely identify what version of the source code produced it, since it's unique.

@BenWibking
Copy link
Collaborator Author

@markkrumholz Let me know if this clarifies things.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 10, 2025
@markkrumholz
Copy link
Collaborator

@markkrumholz Let me know if this clarifies things.

Looks good to me.

auto-merge was automatically disabled February 10, 2025 23:13

Pull request was closed

@markkrumholz markkrumholz reopened this Feb 10, 2025
@markkrumholz
Copy link
Collaborator

Sorry, meant to close the conversation, not the PR.

@BenWibking BenWibking enabled auto-merge February 10, 2025 23:16
@BenWibking BenWibking added this pull request to the merge queue Feb 11, 2025
Merged via the queue into development with commit a1413ac Feb 11, 2025
30 of 37 checks passed
@BenWibking BenWibking deleted the BenWibking/save-git-commit branch February 12, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add git commit hash to metadata.yaml
2 participants