search

deel-ai / deel-lip

Build and train Lipschitz constrained networks: TensorFlow implementation of k-Lipschitz layers
https://deel-ai.github.io/deel-lip/
MIT License
89 stars 10 forks source link

Too restrictive ModuleNotFoundError exception in convolutional #79

Closed Algue-Rythme closed 1 year ago

Algue-Rythme commented 1 year ago

Version

Tensorflow: 2.13 Deel.lip 1.4.0

Issue

In file : https://github.com/deel-ai/deel-lip/blob/master/deel/lip/layers/convolutional.py

There is the code:

try:
    from keras.utils import conv_utils  # in Keras for TF >= 2.6
except ModuleNotFoundError:
    from tensorflow.python.keras.utils import conv_utils  # in TF.python for TF <= 2.5

In tensorflow>=2.13 the new path for conv_utils is tensorflow.python.keras.utils. Unfortunately, attempting to import with from keras.utils import conv_utils does not yield a ModuleNotFoundError becasue the module exists: instead, it simply yield a ImportError.

Since ImportError is not ModuleNotFoundError the except does not catch anything.

Solution

However observe that per this post that ModuleNotFoundError is an instance of ImportError , therefore we should replace the exception with a more general one.

cofri commented 1 year ago

Hi @Algue-Rythme, TensorFlow indeed changed (again) the way conv_utils is imported... @danibene suggested in a previous PR to fix this issue (https://github.com/deel-ai/deel-lip/pull/76/commits/04b03f871c6c8ff043881d55bb66525a47a25e4d). His commit was integrated in a broader PR #76 with corrections for other failures introduced by TF 2.13.

However, we didn't notice that ModuleNotFoundError is a subclass of ImportError. We can simplify the open PR #76 with your proposition.

thib-s commented 1 year ago

Closing this issue as the suggested fix has been merged in #76 . Thank for the contribution !