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

Drop operation and seek_iai benches #388

Merged
merged 4 commits into from
Oct 24, 2023
Merged

Drop operation and seek_iai benches #388

merged 4 commits into from
Oct 24, 2023

Conversation

mmarx
Copy link
Member

@mmarx mmarx commented Oct 23, 2023

  • iai is unmaintained and breaks with valgrind-3.21. The seek_iai bench exercises the same code as the seek bench.
  • the operations bench pulls in polars, which is a huge dependency that regularly breaks its API on (minor) version bumps. Furthermore, the bench itself uses internal parts from the nemo-physical API and thus also regularly requires changes, often only caught in CI, since the bench runs impractically long to actually be useful (still not finished after an hour on Wille – edit: failed after 1:56:00 since test-files/bench-data/aux-split/aux-0.csv was not available), and the exercised code is now covered by the end-to-end tests. It also does not help that non-redistributable test data is required for the bench to successfully execute.

The `operations` bench is no longer useful, yet pulls in polars, a
huge dependency. Back when we did not yet have a full reasoner
implemented, verifying correctness of our joins by comparing with the
polars implementation was useful. This is now covered by, among other
things, our end-to-end tests. Polars is not only a huge dependency
taking a lot of time to compile, but also breaks its API on minor
version bumps (which is not technically a semver violation, since it's
still 0.x, but requires changes to the bench code). Furthermore,
the bench uses internal Nemo APIs and thus also frequently requires
changes, which is often only discovered in CI, since the bench takes
much too long to run regularly. Lastly, it's not pratically useable
for external people, since it depends on test data that is not freely
redistributable.
`iai` is unmaintained and breaks with `valgrind-3.21`.
@mmarx mmarx added enhancement New feature or request technical-debt Refactoring for an implementation that was good enough at the time dependency Something about our dependencies labels Oct 23, 2023
@mmarx mmarx added this to the Release 0.4.0 milestone Oct 23, 2023
@mmarx mmarx self-assigned this Oct 23, 2023
Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/898cb2064b6e98b8c5499f37e81adbdf2925f7c5' (2023-10-13)
  → 'github:NixOS/nixpkgs/5550a85a087c04ddcace7f892b0bdc9d8bb080c8' (2023-10-21)
• Updated input 'nixpkgs-unstable':
    'github:NixOS/nixpkgs/ca012a02bf8327be9e488546faecae5e05d7d749' (2023-10-16)
  → 'github:NixOS/nixpkgs/7c9cc5a6e5d38010801741ac830a3f8fd667a7a0' (2023-10-19)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/a2ccfb2134622b28668a274e403ba6f075ae1223' (2023-10-18)
  → 'github:oxalica/rust-overlay/a3e829c06eadf848f13d109c7648570ce37ebccd' (2023-10-22)
@matzemathics
Copy link
Collaborator

edit: failed after 1:56:00 since test-files/bench-data/aux-split/aux-0.csv was not available

haha, how long has it been since someone ran that?

@mmarx
Copy link
Member Author

mmarx commented Oct 23, 2023

haha, how long has it been since someone ran that?

too long, but that was basically my fault for not uncompressing the file. In any case, two hours is way too long for any single bench.

@matzemathics
Copy link
Collaborator

matzemathics commented Oct 23, 2023

Finally that should be rectified when #311 is implemented.

Until then we might as well drop this chunk of code, when nobody ever runs it.

@matzemathics matzemathics merged commit 27905b3 into main Oct 24, 2023
7 checks passed
@mmarx mmarx deleted the drop-benches branch October 24, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependency Something about our dependencies enhancement New feature or request technical-debt Refactoring for an implementation that was good enough at the time
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants