Skip to content

GitHub actions regression test #499

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

Open
wants to merge 58 commits into
base: main
Choose a base branch
from
Open

GitHub actions regression test #499

wants to merge 58 commits into from

Conversation

MarkWolters
Copy link
Collaborator

@MarkWolters MarkWolters commented Jul 14, 2025

This PR introduces a new github workflow and associated code to perform automated regression testing across branches. The workflow can be triggered manually with the branches to compare included as inputs. It is also triggered to run automatically when a PR to main is opened, in which case it will run a regression test comparing the requested branch with main.

Metrics compared include QPR, average latency, and recall at k10. See attached for an example of the output.
benchmark_report.zip

HIGHER_IS_BETTER = ["QPS", "Recall@10"]
LOWER_IS_BETTER = ["Mean Latency"]

class BenchmarkData:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest leaving two blank lines between top-level functions and two blank lines between the import statements and other code: https://stackoverflow.com/questions/2953250/python-pep8-blank-lines-convention

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done and commited

@@ -0,0 +1,183 @@
/*
Copy link
Collaborator

@marianotepper marianotepper Jul 15, 2025

Choose a reason for hiding this comment

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

Should we tidy up the main example directory? IMO, it is getting polluted. Maybe we should add a "tools" directory or similar. We can do a separate PR for this, no need to do it here as there are other files that should probably go there too and that have nothing to do with this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted and agreed

@marianotepper
Copy link
Collaborator

I suggest editing the description of this PR with a high-level explanation of what it contains/does. It would be great if it also contained an example of the output.

@MarkWolters MarkWolters marked this pull request as ready for review July 16, 2025 15:49
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.

2 participants