Skip to content

Moved test_fit_simple_linear_model function out of run_all_in_graph_and_eager_mode. #1332

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

Merged
merged 1 commit into from
Mar 23, 2020
Merged

Moved test_fit_simple_linear_model function out of run_all_in_graph_and_eager_mode. #1332

merged 1 commit into from
Mar 23, 2020

Conversation

gabrieldemarmiesse
Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse commented Mar 18, 2020

I didn't add the fixture to test in eager mode because running this test in eager mode takes 40seconds compared to 5 seconds with tf.function.

See #1328

@bot-of-gabrieldemarmiesse

@shreyashpatodia

You are owner of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@0xshreyash
Copy link
Contributor

Did you just move the code here?

@gabrieldemarmiesse
Copy link
Member Author

I took it out of the class to avoid making it run twice in graph mode. We're trying to get rid of the @run_all_in_graph_and_eager_mode

@0xshreyash
Copy link
Contributor

Ah okay. Thanks!

@gabrieldemarmiesse
Copy link
Member Author

Thank you for the review!

Copy link
Member

@Squadrick Squadrick left a comment

Choose a reason for hiding this comment

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

LGTM

@Squadrick Squadrick merged commit c799599 into tensorflow:master Mar 23, 2020
@gabrieldemarmiesse
Copy link
Member Author

Thanks for the reviews @Squadrick, much appreciated!

jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants