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

Added the test cases for tf.distribute.MirroredStrategy() to the test_runner #115

Closed
wants to merge 8 commits into from

Conversation

chamorajg
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes Contributor signed Google CLA label Jun 8, 2019
@codecov-io
Copy link

codecov-io commented Jun 20, 2019

Codecov Report

Merging #115 into master will decrease coverage by 0.52%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #115      +/-   ##
========================================
- Coverage   90.53%    90%   -0.53%     
========================================
  Files          31     31              
  Lines        2726   2742      +16     
  Branches      434    438       +4     
========================================
  Hits         2468   2468              
- Misses        179    195      +16     
  Partials       79     79
Impacted Files Coverage Δ
adanet/core/estimator_distributed_test_runner.py 73.64% <0%> (-10.43%) ⬇️

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 cfb357a...62b3050. Read the comment docs.

Copy link
Contributor

@cweill cweill left a comment

Choose a reason for hiding this comment

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

Great work! This is not an easy thing to get working, considering distributed strategy is pretty new code.

adanet/core/estimator_distributed_test_runner.py Outdated Show resolved Hide resolved
adanet/core/estimator_distributed_test_runner.py Outdated Show resolved Hide resolved
adanet/core/estimator_distributed_test_runner.py Outdated Show resolved Hide resolved
adanet/core/estimator_distributed_test_runner.py Outdated Show resolved Hide resolved
adanet/core/estimator_distributed_test_runner.py Outdated Show resolved Hide resolved
…t uses TF_CONFIG here and changes to keep it close to the source version.
@cweill
Copy link
Contributor

cweill commented Jun 21, 2019

Looks good to me. I'll merge it on our side.

@chamorajg
Copy link
Contributor Author

@cweill thanks :)

@cweill cweill self-assigned this Jun 21, 2019
cweill pushed a commit that referenced this pull request Jun 24, 2019
…_runner | Closes PR #115.

Copybara import of the project:

  - 0df5fc6 Changed the protobuf version to 3.6.1 to accomodate lates... by chandramouli <[email protected]>
  - 64985e0 Merge branch 'master' into master by Charles Weill <[email protected]>
  - 3d8cd3a Merge pull request #1 from tensorflow/master by chandramoulirajagopalan <[email protected]>
  - b21ed49 Added tf.distribute.MirroredStrategy() test to estimator_... by chandramouli <[email protected]>
  - 7116db2 Using MultiWorkerMirroredStrategy instead of MirroredStra... by chandramouli <[email protected]>
  - b0c21af Using MultiWorkerMirroredStrategy instead of MirroredStra... by chandramouli <[email protected]>
  - 62b3050 Merge branch 'master' into master by chandramoulirajagopalan <[email protected]>
  - 8185883 Merge 62b3050 into cfb35... by chandramoulirajagopalan <[email protected]>

PiperOrigin-RevId: 254859026
@cweill
Copy link
Contributor

cweill commented Jun 24, 2019

This is now merged in master, so I'm closing this PR. Thanks for this!

@cweill cweill closed this Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor signed Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants