Add bitinfo codec - Githubissues

thodson-usgs commented 5 months ago

Add a new bitinfo codec, which reimplements a numpy-based version of the bitrounding algorithm from the Julia package BitInformation.jl (and the Python package xbitinfo). When used with ds.to_zarr, the codec would compute the real information chunk-wise, which yields better results for data with variable information content.

import xarray as xr
ds = xr.tutorial.open_dataset("air_temperature")

from numcodecs import Blosc, BitInfo
compressor = Blosc(cname="zstd", clevel=3)
filters = [BitInfo(info_level=0.99)]

encoding = {"air": {"compressor": compressor, "filters": filters}}

ds.to_zarr('xbit.zarr', mode="w", encoding=encoding)

TODO:

[x] Unit tests and/or doctests in docstrings
[x] Tests pass locally
[x] Docstrings and API docs for any new/modified user-facing classes and functions
[x] Changes documented in docs/release.rst
[x] Docs build locally
[ ] GitHub Actions CI passes
[ ] Test coverage to 100% (Codecov passes)

thodson-usgs commented 5 months ago

Thanks, @martindurant, I'll work on example, but as I dig into this, I realize that we can't pass a function in this way: https://github.com/zarr-developers/zarr-python/blob/main/zarr/util.py#L56-64 will lead to

TypeError: Object of type function is not JSON serializable

I'd hoped this would work without modifying zarr-python, but for now, I'll go ahead and make a small modification and continue testing.

martindurant commented 5 months ago

Ah, the default serialisation of the codecs, as dumped into the zarr metadata, looks at the __dict__ of the instance, and so is catching the function. Prefixing the attribute with "_" would solve this, or you could override get_config. There is actually a good argument for wanting to serialise the full state, since you can create an array without filling it or fill only part and come back to it later. I don't know how you would approach that without storing some indicator of the function in the metadata.

thodson-usgs commented 5 months ago

Those are both great suggestions. I'll advance one more: add a stripped down xbitinfo to the bitround codec. The implementation looks fairly minimal. Let's see what comes from observingClouds/xbitinfo/issues/257 then reassess.

joshmoore commented 5 months ago

A heads up that depending on custom code will impact the portability of the codec across language implementations.

thodson-usgs commented 5 months ago

observingClouds/xbitinfo/issues/257: their preference is my original suggestion. Potentially yielding something like:

from xbitinfo import helper_function
from numcodecs import Blosc, BitRound
compressor = Blosc(cname="zstd", clevel=3)
filters = [BitRound(helper_function)]

encoding = {"precip": {"compressor": compressor, "filters": filters}}
ds.to_zarr(<file_name>, encoding=encoding)

thodson-usgs commented 5 months ago

@martindurant, I tried your suggestion about prefixing, but I'm getting the general impression this approach won't work. In this example

from xbitinfo import helper_function
from numcodecs import Blosc, BitRound
compressor = Blosc(cname="zstd", clevel=3)
filters = [BitRound(helper_function)]

encoding = {"precip": {"compressor": compressor, "filters": filters}}
ds.to_zarr(<file_name>, encoding=encoding)

the object created by BitRound(helper_function) isn't actually passed through to the filtering step, only the dictionary. The filter is recreated from that dictionary before it is applied. In other words, if I prefix with _, that object won't get passed through the dictionary, and won't be accessible at filter time.

If that impression is correct, then reimplementing a basic bitinformation algorithm in numcodecs might be the simplest route.

martindurant commented 5 months ago

if I prefix with _, that object won't get passed through the dictionary, and won't be accessible at filter time.

Ah, sorry

pep8speaks commented 5 months ago

Hello @thodson-usgs! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file numcodecs/__init__.py:

Line 94:1: E402 module level import not at top of file

Comment last updated at 2024-04-30 21:18:13 UTC

thodson-usgs commented 5 months ago

Rather than including this in bitround, lets create a bitinfo codec that calls bitround. I've drafted the PR and solicited feedback from the xbitinfo devs. I will begin writing tests.

thodson-usgs commented 4 months ago

@observingClouds, @martindurant As my last commit indicates, I've reverted the codec to match the functionality of the xbitinfo implementation (albeit all computation will be done chunk-wise when called through ds.to_zarr()) I'm happy to continue working across libraries to improve functionality and reduce duplication; however, further development needs to come from xbitinfo, not the codec. In the meantime, I do not plan any further changes to this PR except any additional tests or bug fixes as needed.