Skip to content

Conversation

NihalHarish
Copy link
Contributor

@NihalHarish NihalHarish commented Oct 19, 2020

Description of changes:

  • This PR is in the draft stage, I need to update tests, refactor and add comments.
  • Adds ability to save nested layers with the use of the _flatten_layers API. See tests/tensorflow2/test_nested_layers.py
  • Adds the ability to save repeated calls to the same layer object with model subclassing by storing the outputs of the layers in a list in the InputOutputSaver object. See tests/tensorflow2/test_model_that_reuses_layers.py
  • Creates a unique name for each layer output and input to make it more intuitive for the user. See tests/tensorflow2/test_concat_layer.py

Style and formatting:

I have run pre-commit install to ensure that auto-formatting happens with every commit.

Issue number, if available

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-io
Copy link

codecov-io commented Oct 19, 2020

Codecov Report

Merging #377 into master will decrease coverage by 2.72%.
The diff coverage is 98.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #377      +/-   ##
==========================================
- Coverage   85.49%   82.76%   -2.73%     
==========================================
  Files          86       86              
  Lines        6514     6539      +25     
==========================================
- Hits         5569     5412     -157     
- Misses        945     1127     +182     
Impacted Files Coverage Δ
smdebug/tensorflow/keras.py 80.03% <97.77%> (-12.29%) ⬇️
smdebug/tensorflow/utils.py 63.45% <100.00%> (-24.88%) ⬇️
smdebug/tensorflow/singleton_utils.py 83.33% <0.00%> (-16.67%) ⬇️
smdebug/tensorflow/callable_cache.py 69.56% <0.00%> (-13.05%) ⬇️
smdebug/tensorflow/collection.py 84.53% <0.00%> (-11.35%) ⬇️
smdebug/tensorflow/tensor_ref.py 82.25% <0.00%> (-6.46%) ⬇️
smdebug/tensorflow/base_hook.py 76.19% <0.00%> (-4.37%) ⬇️
smdebug/tensorflow/session.py 88.46% <0.00%> (-3.37%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 311a6f4...6826d4b. Read the comment docs.

@NihalHarish NihalHarish changed the title [Draft] Save Nested Layers For Rubik Save Nested Layers For Rubik Oct 20, 2020
layer_input = layer_input[0]
layer_input_tensor_name = get_export_name_for_keras(str(layer_name), "input")
layer_name = str(layer_name)
idx = layer_name_dict.get(layer_name, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this be 0 always because this is the first time the dict has been accessed after L820?

self._export_model()
self._exported_model[self.mode] = True

if is_tf_version_2x():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any restriction for eager/non-eager/tf.function?

if isinstance(layer, str):
# Tensor.name is meaningless when eager execution is enabled.
return f"{layer}/{tensor_type}s"
return f"{layer}_{layer_idx}/{tensor_type}_{tensor_idx}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this change how layer/tensor names have looked so far?

Comment on lines +559 to +579
layer_inputs = self.saved_layers[layer_name].layer_input
for layer_idx, tensor in enumerate(layer_inputs):
if isinstance(tensor, list):
tensor_list = tensor
else:
tensor_list = [tensor]
if hasattr(tensor_list[0], "numpy") is False:
self.logger.warning(
"cannot save layer values during forward pass with tf.function"
)
continue
else:
for t_idx, t in enumerate(tensor_list):
export_name = get_export_name_for_keras(
layer_name,
tensor_type="input",
tensor=tensor,
layer_idx=layer_idx,
tensor_idx=t_idx,
)
self._save_tensor_to_file(export_name, t, layer_collection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks similar to what's done for outputs below. possible to make it common?

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.

3 participants