Bump coverage to 100% #1216
Closed
Bump coverage to 100% #1216
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
@saitcakmak has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Codecov Report
@@ Coverage Diff @@
## main #1216 +/- ##
========================================
Coverage 99.99% 100.00%
========================================
Files 118 118
Lines 10383 10383
========================================
+ Hits 10382 10383 +1
+ Misses 1 0 -1
Continue to review full report at Codecov.
|
|
Hmm shouldn't we instead patch the |
|
Yeah, I was being lazy since it is such a trivial piece of code. Updated. |
Summary: 99.99% just doesn't feel right. Pull Request resolved: #1216 Differential Revision: D36322328 Pulled By: saitcakmak fbshipit-source-id: 56134d3ab0dd849b0f4e0a4d700298293c6d4313
|
This pull request was exported from Phabricator. Differential Revision: D36322328 |
esantorella
added a commit
to esantorella/botorch
that referenced
this pull request
Oct 18, 2022
…1216) Summary: X-link: facebook/Ax#1216 ## Motivation Pyre is not smart enough to understand that calling `self.add_module('model', model)` makes `self.model` have the type of `model`, which is true due to some fairly complex underlying logic inherited from `torch.nn.Module`. However, PyTorch is smart enough to properly `add_module` if we just do `self.model = model`. This also works for tensors, but only if the tensor is explicitly registered as a buffer (by name, not necessarily by value) before assignment. ### Have you read the [Contributing Guidelines on pull requests] Yes Pull Request resolved: pytorch#1452 Test Plan: - Unit tests should be unaffected - Pyre error count drops from 1379 to 1309 (down 5%). - Added explicit tests that `_modules` and `_buffers` are properly initialized Differential Revision: D40469725 Pulled By: esantorella fbshipit-source-id: 5b366a34e1f11b96b8ce551603f3b1a5275c5b95
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 24, 2022
Summary: X-link: facebook/Ax#1216 ## Motivation Pyre is not smart enough to understand that calling `self.add_module('model', model)` makes `self.model` have the type of `model`, which is true due to some fairly complex underlying logic inherited from `torch.nn.Module`. However, PyTorch is smart enough to properly `add_module` if we just do `self.model = model`. This also works for tensors, but only if the tensor is explicitly registered as a buffer (by name, not necessarily by value) before assignment. ### Have you read the [Contributing Guidelines on pull requests] Yes Pull Request resolved: #1452 Test Plan: - Unit tests should be unaffected - Pyre error count drops from 1379 to 1309 (down 5%). - Added explicit tests that `_modules` and `_buffers` are properly initialized Reviewed By: Balandat Differential Revision: D40469725 Pulled By: esantorella fbshipit-source-id: 531cec5b77fc74faf478c4c96f1ceaa596ca8162
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
99.99% just doesn't feel right.