Add transition criteria to GenerationNode #320
Closed
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
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D49509997 |
mgarrard
pushed a commit
to mgarrard/Ax
that referenced
this pull request
Oct 3, 2023
Summary: X-link: facebookresearch/aepsych#320 In this diff we do a few things: (1) Create a TransitionCriterion class: - this class will subsume the CompletionCriterion class and will be a bit more flexible - it has the same child classes + maximumtrialsinstatus subclass, we may add more subclasses or fields later as we further test (2) Create an list of transitioncriterion from the generationstep class: - minimum_trials_observed can be taken care of by the more flexible MinimumTrialsInStatus class - num_trials and enforce_num_trials can be taken care of by the more flexible MaximumTrialsInStatus class (3) adds a doc string to GenNode class - tangential but easy (4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion In following diffs we will: (1) add transition criterion to the repr string + some of the other fields that havent made it yet (2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion (3) add doc strings to everywhere in teh GenNode class (4) add additional unit tests to MaxTrials to bring coverage to 100% (5) skip max trial criterion addition if numtrials == -1 (6) clean up compeletion_criterion class once new ax release can be pinned to aepsych version Reviewed By: lena-kashtelyan Differential Revision: D49509997
|
This pull request was exported from Phabricator. Differential Revision: D49509997 |
crasanders
force-pushed
the
export-D49509997
branch
from
d990ac8
to
e2795f2
Compare
mgarrard
pushed a commit
to mgarrard/Ax
that referenced
this pull request
Oct 4, 2023
Summary: X-link: facebookresearch/aepsych#320 In this diff we do a few things: (1) Create a TransitionCriterion class: - this class will subsume the CompletionCriterion class and will be a bit more flexible - it has the same child classes + maximumtrialsinstatus subclass, we may add more subclasses or fields later as we further test (2) Create an list of transitioncriterion from the generationstep class: - minimum_trials_observed can be taken care of by the more flexible MinimumTrialsInStatus class - num_trials and enforce_num_trials can be taken care of by the more flexible MaximumTrialsInStatus class (3) adds a doc string to GenNode class - tangential but easy (4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion In following diffs we will: (1) add transition criterion to the repr string + some of the other fields that havent made it yet (2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion (3) add doc strings to everywhere in teh GenNode class (4) add additional unit tests to MaxTrials to bring coverage to 100% (5) skip max trial criterion addition if numtrials == -1 (6) clean up compeletion_criterion class once new ax release can be pinned to aepsych version Reviewed By: lena-kashtelyan Differential Revision: D49509997
mgarrard
pushed a commit
to mgarrard/Ax
that referenced
this pull request
Oct 4, 2023
Summary: X-link: facebookresearch/aepsych#320 In this diff we do a few things: (1) Create a TransitionCriterion class: - this class will subsume the CompletionCriterion class and will be a bit more flexible - it has the same child classes + maximumtrialsinstatus subclass, we may add more subclasses or fields later as we further test (2) Create an list of transitioncriterion from the generationstep class: - minimum_trials_observed can be taken care of by the more flexible MinimumTrialsInStatus class - num_trials and enforce_num_trials can be taken care of by the more flexible MaximumTrialsInStatus class (3) adds a doc string to GenNode class - tangential but easy (4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion In following diffs we will: (1) add transition criterion to the repr string + some of the other fields that havent made it yet (2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion (3) add doc strings to everywhere in teh GenNode class (4) add additional unit tests to MaxTrials to bring coverage to 100% (5) skip max trial criterion addition if numtrials == -1 (6) clean up compeletion_criterion class once new ax release can be pinned to aepsych version Reviewed By: lena-kashtelyan Differential Revision: D49509997
mgarrard
pushed a commit
to mgarrard/Ax
that referenced
this pull request
Oct 4, 2023
Summary: X-link: facebookresearch/aepsych#320 In this diff we do a few things: (1) Create a TransitionCriterion class: - this class will subsume the CompletionCriterion class and will be a bit more flexible - it has the same child classes + maximumtrialsinstatus subclass, we may add more subclasses or fields later as we further test (2) Create an list of transitioncriterion from the generationstep class: - minimum_trials_observed can be taken care of by the more flexible MinimumTrialsInStatus class - num_trials and enforce_num_trials can be taken care of by the more flexible MaximumTrialsInStatus class (3) adds a doc string to GenNode class - tangential but easy (4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion In following diffs we will: (1) add transition criterion to the repr string + some of the other fields that havent made it yet (2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion (3) add doc strings to everywhere in teh GenNode class (4) add additional unit tests to MaxTrials to bring coverage to 100% (5) skip max trial criterion addition if numtrials == -1 (6) clean up compeletion_criterion class once new ax release can be pinned to aepsych version Reviewed By: lena-kashtelyan Differential Revision: D49509997
facebook-github-bot
pushed a commit
to facebook/Ax
that referenced
this pull request
Oct 4, 2023
Summary: X-link: facebookresearch/aepsych#320 Pull Request resolved: #1887 In this diff we do a few things: (1) Create a TransitionCriterion class: - this class will subsume the CompletionCriterion class and will be a bit more flexible - it has the same child classes + maximumtrialsinstatus subclass, we may add more subclasses or fields later as we further test (2) Create an list of transitioncriterion from the generationstep class: - minimum_trials_observed can be taken care of by the more flexible MinimumTrialsInStatus class - num_trials and enforce_num_trials can be taken care of by the more flexible MaximumTrialsInStatus class (3) adds a doc string to GenNode class - tangential but easy (4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion In following diffs we will: (1) add transition criterion to the repr string + some of the other fields that havent made it yet (2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion (3) add doc strings to everywhere in teh GenNode class (4) add additional unit tests to MaxTrials to bring coverage to 100% (5) skip max trial criterion addition if numtrials == -1 (6) clean up compeletion_criterion class once new ax release can be pinned to aepsych version Reviewed By: lena-kashtelyan Differential Revision: D49509997 fbshipit-source-id: f498eca7a251cbeca2728068bdd9d9b10a50c2c4
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:
X-link: facebook/Ax#1887
In this diff we do a few things:
(1) Create a TransitionCriterion class:
(2) Create an list of transitioncriterion from the generationstep class:
(3) adds a doc string to GenNode class - tangential but easy
(4) updates the type of completion_criteria of GenerationStep from CompletionCriterion to TransitionCriterion
(5) clean up compeletion_criterion class bc we don't need it anymore
In following diffs we will:
(1) add transition criterion to the repr string + some of the other fields that havent made it yet
(2) begin moving the functions related to completing the step up to node and leveraging the transition criterion for checks instead of indexes -- this is where we may need to add additional fields to transitioncriterion
(3) add doc strings to everywhere in teh GenNode class
(4) add additional unit tests to MaxTrials to bring coverage to 100%
(5) skip max trial criterion addition if numtrials == -1
Reviewed By: lena-kashtelyan
Differential Revision: D49509997