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 KeyError when validation_data was given as a dict #30258

Merged
merged 3 commits into from Jul 25, 2019

Conversation

yongtang
Copy link
Member

This fix tries to address the issue raised in #30122 where a KeyError was thrown when validation_data was given as a dict during the mode.fit. This fix fixes the issue.

This fix fixes #30122.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

This fix tries to address the issue raised in 30122 where
a KeyError was thrown when validation_data was given as a dict
during the mode.fit. This fix fixes the issue.

Thisfix fixes 30122.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@tensorflow-bot tensorflow-bot bot added the size:S CL Change Size: Small label Jun 30, 2019
@gbaned gbaned self-assigned this Jul 1, 2019
@gbaned gbaned added the comp:keras Keras related issues label Jul 1, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 1, 2019
@gbaned gbaned requested a review from karmel July 1, 2019 05:10
@yongtang yongtang added the kokoro:force-run Tests on submitted change label Jul 7, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 7, 2019
@karmel karmel requested review from qlzh727 and removed request for karmel July 8, 2019 16:45
@@ -207,7 +207,8 @@ def model_iteration(model,
val_samples_or_steps = validation_steps
else:
# Get num samples for printing.
val_samples_or_steps = val_inputs and val_inputs[0].shape[0] or None
vals = val_inputs.values() if isinstance(val_inputs, dict) else val_inputs
val_samples_or_steps = vals and vals[0].shape[0] or None
Copy link
Member

Choose a reason for hiding this comment

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

probably skip the dict check and use nest.flatten(vals)[0].shape[0] here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @qlzh727, the PR has been updated.

@@ -110,6 +110,43 @@ def test_print_info_with_numpy(self, do_validation):
if do_validation:
self.assertIn(", validate on 50 samples", mock_stdout.getvalue())

def test_dict_input(self):
Copy link
Member

Choose a reason for hiding this comment

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

Should be test_dict_validation_input()

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

model = my_model()
model.compile(loss="mae", optimizer="adam")

mock_stdout = six.StringIO()
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the reason to mock the output, are u trying to validate any output here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment. Removed.

PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Jul 8, 2019
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Copy link
Member Author

yongtang commented Jul 9, 2019

Thanks @qlzh727 for the review. The PR has been updated. Please take a look and let me know if there are any other issues.

@yongtang yongtang added the kokoro:force-run Tests on submitted change label Jul 22, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 22, 2019
@yongtang yongtang added the kokoro:force-run Tests on submitted change label Jul 22, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 22, 2019
@yongtang
Copy link
Member Author

Ping @qlzh727, any chance to take a look at the PR?

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jul 22, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 22, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 22, 2019
@tensorflow-copybara tensorflow-copybara merged commit 7dec500 into tensorflow:master Jul 25, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Jul 25, 2019
tensorflow-copybara pushed a commit that referenced this pull request Jul 25, 2019
@yongtang yongtang deleted the 30122-KeyError-dict branch July 25, 2019 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:keras Keras related issues ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

Getting validation steps from dict breaks keras fit TF 2.0.0-beta1
6 participants