nens / threedi-modelchecker

Tool to verify the correctness of a 3Di model
MIT License
0 stars 2 forks source link

Make modelchecker tests work with numpy 2.0 #378

Open margrietpalm opened 1 month ago

elisalle commented 1 month ago

Looked at this with Casper and since GDAL does not support numpy 2, we currently cannot either.

margrietpalm commented 1 month ago

How about locking the numpy version for testing to <2.0 (in https://github.com/nens/threedigrid-builder/blob/master/setup.py#L63)? @caspervdw @leendertvanwolfswinkel

caspervdw commented 1 month ago

We actually have two environments that threedi-modelchecker should run in:

I think it might be best to do an automated test with numpy 2 and without GDAL, and an automated test with the GDAL + numpy versions in use in QGis (maybe using a docker like https://github.com/OSGeo/gdal/pkgs/container/gdal/236116077?tag=ubuntu-small-3.9.1)

This BTW also goes from threedigrid-builder, but not for threedi-tables which should only run in the backend. threedi-schema has no depedency on numpy.

elisalle commented 1 month ago

Many of the tests fail when executed with numpy<2, rasterio, and no gdal; not just gdal tests. All of them are in test_checks_raster.py or test_model_checks.py. Examples are test_checks_raster.py::test_base_get_invalid_local and test_model_checks.py::test_get_model_error_iterator.

margrietpalm commented 1 month ago

How do you mean fail with numpy<2? Why would they fail if no dependency was changed?

elisalle commented 1 month ago

I assume because they are usually run with GDAL installed; when I install pygdal 3.4.1.10 they run fine. My point is that testing with rasterio and numpy 2 but no GDAL might be a challenge, considering that many tests don't even seem to pass with rasterio, numpy 1, and no GDAL.

caspervdw commented 1 month ago

So the modelchecker works in QGis (with GDAL delivered by Qgis) and in the backend ("workers" module in threedi-api). This is the environment where the modelchecker should work: https://github.com/nens/threedi-api/blob/master/workers

I see numpy 1.19, rasterio 1.3.10, and also gdal (https://github.com/nens/threedi-api/blob/bffdb1945efba0397d8e7be3fbfe5df0536f157c/workers/Dockerfile#L13C1-L14C1)

So it seems that all environments use GDAL, and not rasterio.

Now I am confused, why is the whole rasterio there then in modelchecker? @daanvaningen @martijn-siemerink do you have an idea?

elisalle commented 1 month ago

Until this is resolved, I've put in a NumPy 1 pin: https://github.com/nens/threedi-modelchecker/pull/379

caspervdw commented 1 month ago

Apparently it does work, as long as you make sure GDAL is built against numpy 2. The following does the trick:

pip install numpy==2.* && pip install gdal[numpy]==3.4.1 --no-build-isolation

Ref: https://numpy.org/doc/stable/user/building.html#numpy-2-0-specific-advice

When you build wheels for your package using a NumPy 2.x version at build time, those will work with NumPy 1.xx.

For QGis, this issue will for sure be addressed by QGis. We just need to adhere to whathever GDAL/numpy versions they prescribe. For threedi-api, the issue will pop up as soon as threedi-api upgrades to Numpy 2. Probably will take some years ;) For the GH actions of modelchecker, gridbuilder etc.: make sure that GDAL is built against numpy 2 so that tests can run against numpy 1 and numpy 2.

elisalle commented 1 month ago

Managed to get NumPy 2 and GDAL working together in an action. According to the release description of rasterio 1.3.10, the latest release and the first to support NumPy 2:

Wheels for Python versions >= 3.9 will be built using Numpy 2.0.0rc1 and will be compatible with the oldest supported Numpy 1.x. Wheels for Python version 3.8 will be built using the oldest supported version of NumPy and will not be compatible with Numpy 2.

Since Python 3.8 security updates will end on 31 October anyway, this doesn't seem like a major issue.

margrietpalm commented 1 month ago

Numpy 2.0 is only supported by python 3.9 to 3.12 (https://numpy.org/news/) and so any user with 3.8 will get numpy 1.x. If I understand you correctly @elisalle that will still work.