Skip to content

Fix benchmark CI #609

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

Merged
merged 1 commit into from
Mar 28, 2025
Merged

Fix benchmark CI #609

merged 1 commit into from
Mar 28, 2025

Conversation

achaikou
Copy link
Contributor

Current logic is flawed.
Intention of benchmark tests was to build library from two different commits. However only C/C++ part was built separately, python code was not touched.

When current commit's directory was checked out to run the fresh tests, so was other python code: tests were always run on the same python part of the library.

To fix that, build a local wheel from existing code and make sure it is being used instead of local code.

Copy link
Contributor

@yngve793 yngve793 left a comment

Choose a reason for hiding this comment

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

Not sure how to test that the two wheels are different.
Looks good and all tests and checks passes.

Copy link
Contributor Author

@achaikou achaikou left a comment

Choose a reason for hiding this comment

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

Not sure how to test that the two wheels are different.

It got tested for us when I was doing datasource changes 😅
I was very surprised there was no reduction in performance when I was running new benchmarks against slower datasources 😄

Currently you can only check that different wheels are installed.
Run benchmarks manually in Actions and supply older ref, for example c946c3c.
You'll have
Successfully installed segyio-1.9.12
vs
Successfully installed segyio-1.9.13

I've also added prints to assure where segyio is loaded from.
Don't think more could be done.

Current logic is flawed.
Intention of benchmark tests was to build library from two different
commits. However only C/C++ library was built separately, python code
was not touched and included.

When current commit's directory was checked out to run the fresh tests,
so was other python code: tests were always run on the same python part
of the library.

To fix that, build a local wheel from existing code and make sure it is
being used instead of local code.
@achaikou achaikou merged commit 3299427 into equinor:main Mar 28, 2025
25 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.

2 participants