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

Add Zarr benchmarks #51

Merged
merged 16 commits into from
Apr 28, 2024
Merged

Add Zarr benchmarks #51

merged 16 commits into from
Apr 28, 2024

Conversation

oruebel
Copy link
Contributor

@oruebel oruebel commented Apr 26, 2024

Fix #49

This PR makes the following main changes:

  • Update get_object_by_name to support look-up of an object by name even if a file contains duplicate names (i.e., enforce only that the name of the object we are actually looking for is unique)
  • Update core/_streaming.py to add functions for reading Zarr NWB files
  • Add hdmf-zarr to the requirements
  • Add network tracking benchmarks for Zarr (network_tracking_remote_file_reading.py and network_tracking_remote_slicing.py )
  • Add timing benchmarks for Zarr (time_remote_file_reading.py and time_remote_slicing.py)
  • Update to new BaseBenchmark class and parametrize scheme

Note: Currently we should use hdmf-zarr from the dev branch for this PR since the changes to allow us to configure to not use consolidated metadata have been merged but not released yet (or ideally even use hdfm-zarr @ git+https://github.com/hdmf-dev/hdmf-zarr.git@fix/consolidated_open_bug). However, we are planning to a release hdmf-zarr with the relevant fixes on Monday so I only listed hdmf-zarr in the dependencies, rather than setting a particular branch

@oruebel
Copy link
Contributor Author

oruebel commented Apr 26, 2024

Note, I ran a few individual tests (which all worked), but I have not been able to rerun all the benchmarks with the additional tests yet.

@oruebel oruebel marked this pull request as ready for review April 26, 2024 03:35
@oruebel
Copy link
Contributor Author

oruebel commented Apr 28, 2024

@CodyCBakerPhD Please take another look. I've

  • updated read_zarr to not do the fallback if consolidated metadata is not present
  • updated read_zarr_nwbfile to expose the mode parameter (NOTE: hdmf-zarr does the fallback check under the hood)
  • updated the benchmark classes to use the new BaseBenchmark class and parametrization scheme

@CodyCBakerPhD
Copy link
Collaborator

Looks good to me 👍

@CodyCBakerPhD CodyCBakerPhD merged commit ef667c9 into main Apr 28, 2024
2 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the add/zarr_tests branch April 28, 2024 21:25
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.

Add Zarr read tests
2 participants