Skip to content

Conversation

@jo-basevi
Copy link
Contributor

@jo-basevi jo-basevi commented Jul 30, 2025

This PR:

  • Adds a new input to test-repro.yml - config-hash. This is so all experiment directories will be set as
    ${{ vars.EXPERIMENTS_LOCATION }}/${{ github.event.repository.name }}/${{ inputs.config-ref }}/${{ inputs.config-hash }} where config-ref is the branch or tag name and config-hash is the SHA commit hash. This is to make it easier to group commits to a particular branch in a PR (when eventually we set up automation to delete them when PR is closed) and also keep checksums on intermediate commits
  • After the test results are checked for errors (depending on Generate repro test errors when model does not run successfully #173 this should indicate if the model did not run correctly), remove any directories apart from checksum.

Contributes to #175 - this PR currently does not remove anything when config PRs are closed.

TODO:

  • Testing?
  • Double check the checksums file exists in artefact before deleting.
  • Could there be a repo setting to prevent deletion of outputs?

…uccessfully

- Delete outputs when test status is not error (which will indiciate whether the model exited without errors)
- Add optional input for config-hash to allow grouping commits to a branch (to make it easier to compare checksums on intermediate commits and eventually delete all commits associated with a branch)
- Update config-comment-test.yml and test-repro.yml
This might also make issues more clear for scheduled checks on branches that get new commits that fix reproducibility as the comments will point to the older commit
@jo-basevi jo-basevi force-pushed the 175-clean-up-test-outputs branch from 14affd6 to cfa50f9 Compare August 10, 2025 04:35
@jo-basevi jo-basevi requested a review from CodeGat August 11, 2025 00:20
Copy link
Member

@CodeGat CodeGat left a comment

Choose a reason for hiding this comment

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

Forgot about this one - thanks for addressing the comment I had

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.

3 participants