Skip to content
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

fix: Implemented support for loading models with Concatenate layers #1192

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

Jucko13
Copy link
Contributor

@Jucko13 Jucko13 commented Oct 10, 2023

This is my first serious pullrequest so please be gentle :)

The tf.keras.models.load_model() is now able to load models with this layer type.

There was a lot missing but I have a Unet model running in the field that now fully works.

I had some weird troubles with python tensorflow (im using 2.11.1) that savemodel would put the inbound nodes for concatenate layers in an extra nested array. My call in python looks like:

comb = layers.Concatenate(axis=3, name=name+"_concat1")([concat_layer,conv])

This would create the following output in the manifest (notice the extra [ and ]:

{
  "class_name": "Concatenate",
  "config": {
    "name": "EXP_1_concat1",
    "trainable": true,
    "dtype": "float32",
    "axis": 3
  },
  "name": "EXP_1_concat1",
  "inbound_nodes": [
    [
  	[
  	  "CNT_4_conv_2",
  	  0,
  	  0,
  	  {}
  	],
  	[
  	  "EXP_1_conv_0",
  	  0,
  	  0,
  	  {}
  	]
    ]
  ]
},

so thats why i added an extra check in generic_utils.cs for this extra nest. I would love some feedback on the implementation of this check and if it's the correct way to do.

And lastly, since concatenate layers have more than 1 input node we need a List when processing nodes in Functional.FromConfig.cs.

I also added a few tests to make sure that saving and loading a model with concatenate layers is the same (this was not the case before my changes).

model.load_model now supports loading of concatenate layers.
python tensorflow exports concatenate layers in an extra nested array in the manifest so added a check for that in generic_utils.cs.
Concatenate was missing the build=true, this fix prevents the layer being build multiple times.
Concatenate has 2 or more input nodes so List<NodeConfig> was required instead of just NodeConfig in Functional.FromConfig.cs.
Added missing axis JsonProperty attribute for MergeArgs (used by Concatenate)
The loading and saving of a simple model with a Concatenate layer is tested to check if the model is the same after reloading.
Implemented missing axis parameter for np.stack (added some handy tuple calls too like the np.concatenate example).
@Wanglongzhi2001
Copy link
Collaborator

Thank you very much for your contribution!

@Wanglongzhi2001
Copy link
Collaborator

Looks good to me! @Oceania2018

@Jucko13
Copy link
Contributor Author

Jucko13 commented Oct 11, 2023

Do i have to do anything regarding the semantic check that failed? But to fix that I need to rename the commit title?

@Wanglongzhi2001
Copy link
Collaborator

Wanglongzhi2001 commented Oct 11, 2023

Do i have to do anything regarding the semantic check that failed? But to fix that I need to rename the commit title?

The reason for semantic checking error is the incorrect format of your commit title. The ci in TensorFlow.NET obey the angular style, You can change it to fix: Implemented support for loading models with Concatenate layers. But it's just a trivial question, you can discard it.

@Jucko13 Jucko13 changed the title Implemented support for loading models with Concatenate layers fix: Implemented support for loading models with Concatenate layers Oct 11, 2023
@Oceania2018 Oceania2018 merged commit 9e3654b into SciSharp:master Oct 11, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants