Remove parameter constraints when expanding model space in Adapter #3408
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
Summary: By default we expand range parameters in the model space so that the training data include all modelable data, not just those that fall within the box bounds. A similar argument suggests that we should also include training data that violate parameter constraints in the model. They are modelable and can be used to improve predictions at points that satisfy parameter constraints. We do this by simply dropping parameter constraints from the model space, if the model space is specified to be expanded. This will also fix facebook#1568 . The issue there is that generated points violated parameter constraints due to numerical issues, and so then were left out of the training data and repeatedly generated. With this change, those points would be included in the training data and so likely not re-generated. Note, however, that the purpose of this change is not just to fix this issue; it is a change that is right because it better fits the concept of the modeling space. Reviewed By: sdaulton, saitcakmak Differential Revision: D69880066
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D69880066 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3408 +/- ##
=======================================
Coverage 95.99% 95.99%
=======================================
Files 539 539
Lines 52794 52801 +7
=======================================
+ Hits 50677 50684 +7
Misses 2117 2117 ☔ View full report in Codecov by Sentry. |
|
This pull request was exported from Phabricator. Differential Revision: D69880066 |
|
This pull request has been merged in d86a17b. |
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.
Summary:
By default we expand range parameters in the model space so that the
training data include all modelable data, not just those that fall within the
box bounds.
A similar argument suggests that we should also include training data that
violate parameter constraints in the model. They are modelable and can be used
to improve predictions at points that satisfy parameter constraints.
We do this by simply dropping parameter constraints from the model space, if
the model space is specified to be expanded.
This will also fix #1568 . The issue there
is that generated points violated parameter constraints due to numerical
issues, and so then were left out of the training data and repeatedly
generated. With this change, those points would be included in the training
data and so likely not re-generated. Note, however, that the purpose of this
change is not just to fix this issue; it is a change that is right because it
better fits the concept of the modeling space.
Reviewed By: sdaulton, saitcakmak
Differential Revision: D69880066