search

bamler-lab / constriction

Entropy coders for research and production in Python and Rust.
https://bamler-lab.github.io/constriction/
Apache License 2.0
80 stars 6 forks source link

Relax minimal numpy version, if possible #40

Closed kshpytsya closed 9 months ago

kshpytsya commented 10 months ago

I am currently working on maintaining a legacy codebase which for valid reasons[^1] requires numpy (>=1.16.0,<1.19.0). I was about to introduce constriction as a mean to significantly reduce memory consumption by some base models, only to find out that I cannot install it due to version conflict.

Is numpy>=1.19.0 a hard requirement?

[^1]: Old versions of some scientific libraries require old version of numpy. It is impossible to upgrade those libraries because newer versions have introduced incompatible changes to serialized models. We have to support existing models and cannot recreate them.

robamler commented 9 months ago

Yeah, no need to explain yourself for using old versions of python packages, I know painfully well how disastrous the package management situation is in python :). There's no deep reason for requiring numpy ^1.19.0, it's just the oldest version of numpy I've ever tested with.

I just released a new version of constriction (v0.3.3) that reduces the required numpy version to ^1.16. Please let me know if this resolves your problem (you'll need to use python 3.10 or older; for python 3.11, we require numpy ^1.23.2, and for python 3.12 we require numpy ^1.26.1; relaxing those requirements would be much more difficult).

kshpytsya commented 9 months ago

Perfect timing. I was about to do a fork. However, there is a problem, probably due to a possible maturin's bug, which causes wheels to contain Requires-Dist: numpy~=1.19 in METADATA regardless of the target python version. A quick scan of maturin's issue tracker didn't discover anything relevant. So I suppose, someone should open an issue. I believe you to be more suitable as my acquaintance with maturin is cursory. If you do not have time for that, I will try opening an issue myself.

P.S. Maybe I am barking at the wrong tree, but I cannot add constriction to my Poetry project requiring Python 3.8 which has other dependencies requiring numpy<1.19.

kshpytsya commented 9 months ago

P.P.S. It looks like you should change requirements here:

https://github.com/bamler-lab/constriction/blob/4077f33b34bb6b44ea27fd771eb80f94e1a95b80/pyproject.toml#L17

See https://www.maturin.rs/metadata. I have verified that those are the requirements that make it into the wheel metadata, and you have changed only Poetry-specific requirements.

robamler commented 9 months ago

Sorry, I should have double checked this. Thank you for hunting down the bug yourself!

I fixed it in commit 3f2a96c and just released a new version (this time I did actually look into the wheel's metadata and verified it now contains "numpy~=1.16". Please let me know if this works for you.

kshpytsya commented 9 months ago

Great! I can confirm that constriction==0.3.4 is installable by Poetry==1.7.1 into Python==3.8.16 along with numpy==1.18.5. I have also verified that encoder works.

I minor detail – I am unsure whether it is worth a separate issue – is the following warning from Poetry:

Warning: Validation of the RECORD file of constriction-0.3.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl failed. Please report to the maintainers of that package so they can fix their build process. Details:
In /home/uken/.cache/pypoetry/artifacts/bf/7f/3e/dafe24f40a54f14858cdb770e83c4fc076d2a8e86141bb380a25393718/constriction-0.3.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl, LICENSE.html is not mentioned in RECORD
robamler commented 9 months ago

Great, thanks! I opened #43 for the issue about validating python wheels. The warning is no reason for concern, but I should indeed fix it to prevent the warning message.