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

Remove _check_previous_required_observation method #1999

Closed
wants to merge 2 commits into from

Commits on Nov 28, 2023

  1. Replace calls to trial related step specific methods in GenerationStr…

    …ategy (facebook#2002)
    
    Summary:
    
    This diff removes the last uses of num_remaining_trials_until_max_parallelism(), num_trials, num_can_complete, and enforce_num_trials from the GenerationStrategy file. This can be done bc we can compute all the necessary info on GenerationNode now. And, since we want GenerationStrategy to accept GenerationNodes and GenerationSteps we cannot have any leftover calls from the GenerationStep level.
    
    Reviewed By: lena-kashtelyan
    
    Differential Revision: D51172395
    Mia Garrard authored and facebook-github-bot committed Nov 28, 2023
    Configuration menu
    Copy the full SHA
    c89ff93 View commit details
    Browse the repository at this point in the history
  2. Remove _check_previous_required_observation method (facebook#1999)

    Summary:
    
    This method is redundant and checks for a state that we should never reach. In the process of updating to GenerationNode based GS, maintaining this method would entail keeping track of previous nodes. The tech debt of doing so outweighs the potential reward of keeping the function. Also I think since use_update has been deprecated this function is also less relevant.
    
    Reviewed By: lena-kashtelyan
    
    Differential Revision: D51420534
    Mia Garrard authored and facebook-github-bot committed Nov 28, 2023
    Configuration menu
    Copy the full SHA
    69fa2f7 View commit details
    Browse the repository at this point in the history