-
Notifications
You must be signed in to change notification settings - Fork 444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TF] First changes on the road to Keras v3 #1724
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1724 +/- ##
==========================================
- Coverage 96.47% 96.43% -0.04%
==========================================
Files 164 164
Lines 7823 7823
==========================================
- Hits 7547 7544 -3
- Misses 276 279 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@odulcy-mindee I see two options:
What do you prefer ? |
Next steps:
|
@@ -184,6 +184,8 @@ def main(args): | |||
|
|||
# Resume weights | |||
if isinstance(args.resume, str): | |||
# Build the model first to load the weights | |||
_ = model(tf.zeros((1, args.input_size, args.input_size, 3)), training=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need to set training=True
somewhere after load_weights
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that's only to build the model training=True is handled in the training scripts :)
And i hope i can remove this part with the next follow up PR where i need to implement the def build
methods for most of the layers 😅
if not params_path.is_dir() or overwrite: | ||
with ZipFile(archive_path, "r") as f: | ||
f.extractall(path=params_path) | ||
# Build the model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odulcy-mindee
I hope i can remove this in a follow up PR if all models have build
and get_config
methods where it's required
@@ -184,6 +184,8 @@ def main(args): | |||
|
|||
# Resume weights | |||
if isinstance(args.resume, str): | |||
# Build the model first to load the weights | |||
_ = model(tf.zeros((1, args.input_size, args.input_size, 3)), training=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that's only to build the model training=True is handled in the training scripts :)
And i hope i can remove this part with the next follow up PR where i need to implement the def build
methods for most of the layers 😅
d181043
to
d2494ed
Compare
@@ -56,6 +56,8 @@ tf = [ | |||
# cf. https://github.com/mindee/doctr/pull/1461 | |||
"tensorflow>=2.11.0,<2.16.0", | |||
"tf2onnx>=1.16.0,<2.0.0", # cf. https://github.com/onnx/tensorflow-onnx/releases/tag/v1.16.0 | |||
# TODO: This is a temporary fix until we can upgrade to a newer version of tensorflow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temp pin numpy until we can upgrade tensorflow to fix the issue with poetry
and the failing API test (Will have effect after merging)
This PR:
is part of a series of PR's
basic elements to go further forward (only .weights.h5 and .keras (full model) - loading possible in keras v3)
change to keras imports
Finally: enable fine tuning tf models with
pretrained
also if the vocab / classes differs !! 💯 (same we have already a long time ago added to PT - which wasn't possible with loading from/weights
in TF)all models works as expected
Next steps:
build
andget_config
methods for proper loading & saving (serialization)Any feedback is welcome 🤗
NOTE: It's still fully backwards compatible - but marking as breaking change because that will be the last release where you can load
/weights
- so we have to mention this in the next release notes@odulcy-mindee models upload :)