search

juglab / n2v

This is the implementation of Noise2Void training.
Other
385 stars 107 forks source link

Update requirements #127

Closed jdeschamps closed 1 year ago

jdeschamps commented 1 year ago

I tested all notebooks with TF 2.7 and 2.10 (latest), and CSBDeep 0.7.2 (latest) in Python 3.9.

Here is roughly how I installed the environment:

conda install -c conda-forge cudatoolkit=11.2 cudnn
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$CONDA_PREFIX/lib/
pip install tensorflow
pip install -e .
jdeschamps commented 1 year ago

A lot of changes are also due to my IDE reformatting the file... But @tibuch maybe you could just have a look at https://github.com/juglab/n2v/pull/127/commits/461505bc04970b728f8beadb258b3b50ac4bf353 because that is the only change to the code base.

jdeschamps commented 1 year ago

We also need to update the installation instructions. I will test this later today!

This page recommends to also specify cudnn version during installation.

tibuch commented 1 year ago

It is probably a good idea to follow this recommendation.

Could you ping me once you updated the instructions? Then I can test-run it from start to finish :v:

jdeschamps commented 1 year ago

It is probably a good idea to follow this recommendation.

Could you ping me once you updated the instructions? Then I can test-run it from start to finish v

Done (https://github.com/juglab/n2v/pull/127/commits/0c0f7905e97481f43412364668127e52d5cbcf84).

I chose to follow the TF guidelines and link to it. We could also just say use tensorflow==2.7 or tensorflow==2.10 for safety.

tibuch commented 1 year ago

Thank you @jdeschamps for all the work!

I was able to install everything and run the tests and example notebooks as they are.

I did not run the functional test. They just take too long.

I also tested the N2V2 functionality by setting

Training and prediction works well for the 2D case (in 3D the model is not initialized with an NotImplementedError #129). However, model.export does not work with the current blurpool implementation (see #128).

jdeschamps commented 1 year ago

@tibuch oh good catch.

I have also not tried the functional tests, I have excluded them from tox testenv. I will start them on a virtual machine today and check them tomorrow.

One more thing. It seems that on my RedHat machine, I need to set the export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$CONDA_PREFIX/lib/ every time I start the environment new (e.g.: you close the terminal and restart it). Do you also have this or is it RedHat specific? It may be due to newer versions of TF... If that is the case, that is super annoying and we should consider adding the export to the notebooks.

tibuch commented 1 year ago

This should be fixed by running this once in the acitvate env:

mkdir -p $CONDA_PREFIX/etc/conda/activate.d
echo 'export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$CONDA_PREFIX/lib/' > $CONDA_PREFIX/etc/conda/activate.d/env_vars.sh
jdeschamps commented 1 year ago

This should be fixed by running this once in the acitvate env:

mkdir -p $CONDA_PREFIX/etc/conda/activate.d
echo 'export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$CONDA_PREFIX/lib/' > $CONDA_PREFIX/etc/conda/activate.d/env_vars.sh

I actually had missed your comments because I didn't refresh the page. Perfect!

jdeschamps commented 1 year ago

The functional tests run through without errors btw.

jdeschamps commented 1 year ago

I merged here the fix for the model export (https://github.com/juglab/n2v/issues/128). However there is still the bug that we cannot save twice in a row. I believe this is due to the way CSBDeep saves the model to .pb format (see https://github.com/CSBDeep/CSBDeep/issues/62).

Since the export works, I would suggest to create an issue about this bug to keep track of it and see what becomes of it in the next CSBDeep version. In the mean time since it does not prevent users from using N2V notebooks, I would go ahead with the new version release.