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

regression tests & benchmark #929

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

epolack
Copy link
Collaborator

@epolack epolack commented Dec 12, 2023

  • If the action is to merge on master, maybe compare to previous known release? Otherwise, makes mostly sense only for PR.
  • Locally, the results are consistent. On the CI the target is way to fast, maybe because for the action to make sense, I have to run against the exact same version. I tested with retune, and the results are more logical; so I am keeping it that way for now:
    no-retune v.s. retune
  • Separate load testing from regression?
  • For the humongous tests it still need some work for it to be flexible and easily usable; e.g., allow user to chose baseline, allow to use retune option.
  • Silence tests (Using Suppressor to silence tests #957)
  • Add regression tests for Julia Nightly?
  • Document for the developer.
  • For humongous tests, retune not useful
  • Have @benchmarkable directly in code?
  • Density and ad tests are too inconsistent between run. Increase load.
  • use DFTK-testproblems as submodule? Modify CI with
    with:
      submodules: true
    
  • The github-bot does not comment on the PR, even if indicated as so. May be a permission issue, but do we really need this feature? Also, will be messed up if two concurrent jobs.

Also: added @testset to continue even if some tests fails…

@epolack epolack linked an issue Dec 12, 2023 that may be closed by this pull request
@epolack epolack force-pushed the pkgbenchmark branch 7 times, most recently from 61daf9d to 59f1cc8 Compare December 12, 2023 11:24
- name: Run benchmarks
# Remove baseline once merged. Regression tests will only work after this is merged
# in master.
run: julia -e 'using BenchmarkCI; BenchmarkCI.judge(; baseline="HEAD", retune=true)'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not forget.

benchmark/run.jl Outdated
Comment on lines 11 to 12
# Remove target once merged. Regression tests will only work after this is merged in master.
BenchmarkCI.judge(; baseline="HEAD~1")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not forget.

@epolack epolack force-pushed the pkgbenchmark branch 21 times, most recently from 48e9277 to 960b0b4 Compare December 12, 2023 13:57
@@ -116,6 +116,7 @@ ASEconvert = "3da9722f-58c2-4165-81be-b4d7253e8fd2"
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
AtomsIO = "1692102d-eeb4-4df9-807b-c9517f998d44"
AtomsIOPython = "9e4c859b-2281-48ef-8059-f50fe53c37b0"
BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

May not be really useful. But would allow to run some regression tests as part as the standard test set. Otherwise, it will crash.
Remove?

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.

Use BenchmarkCI to automatically detect major runtime regressions
1 participant