similarity icon indicating copy to clipboard operation
similarity copied to clipboard

Convnext architecture dev

Open Lorenzobattistela opened this issue 1 year ago • 4 comments

Implementing ConvNeXt architecture referred in this paper and #353 .

Reviewer: @owenvallis

This PR re implements #354 but based on the development branch to fix some test and formatting issues.

Lorenzobattistela avatar Aug 28 '23 13:08 Lorenzobattistela

Test errors on CI: ImportError: cannot import name 'convnext' from 'tensorflow.keras.applications' Maybe convnext wasnt a keras.applications module in the version of tensorflow CI is using. (https://www.tensorflow.org/api_docs/python/tf/keras/applications/convnext)

In my local, all tests run ok. My TF version is 2.13.0

Lorenzobattistela avatar Aug 28 '23 14:08 Lorenzobattistela

Hi @Lorenzobattistela, looks like convnext wasn't introduced until TF v2.10. I wonder if we can do a TF version check for this? The other option is we could provide a general architecture class that accepts a tf.keras.application as input and wraps it. I'm not sure if it would be simple to apply to all applications, but would be cleaner for the package and avoids the version issue altogether.

owenvallis avatar Sep 08 '23 04:09 owenvallis

So, @owenvallis I thought about what you said. Anyway, I updated the code to do some version checking on test (to skip if tf < 2.10), and maybe we can add some version checking to inform a more useful error to the user if it tries to use it with a minor tf version.

However, I think the refactoring path to a wrapper for keras applications is the best approach. I'm willing to work on this, will start refactoring it.

It is up to you to merge or not this, maybe we can use this as a "hotfix" and then refactor to something better. Thanks for the review.

Lorenzobattistela avatar Sep 08 '23 22:09 Lorenzobattistela

@owenvallis something is going wrong with isort, but i did ran it

Lorenzobattistela avatar Sep 12 '23 13:09 Lorenzobattistela