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.

Sign up for GitHub

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

Jump to bottom

Add should_skip setting to input constructors #2894

Closed
wants to merge 3 commits into from
Closed

Add should_skip setting to input constructors #2894

wants to merge 3 commits into from

Commits on Oct 17, 2024

  1. Add should_skip property to GenerationNode (facebook#2891) 

    Summary:

There are some cases where the input constructor returns n=o, specifically for repeat arms, this is currently causing an issue in generaiton strategy gen method because if we don't generate from a node we can't meet the TC, and move forward, however, we don't actually want to generate from that node.

This simply exposes a prop on gen node to expose this, and default it to false

Following diffs will:
- update TC.is_met() to accept a GenNode instead of just the name
- update input constructor to properly set should skip
- update the gs to appropiately handle resetting to false after a gen is completed

Reviewed By: lena-kashtelyan

Differential Revision: D64444258
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    406e3e4 View commit details
    Browse the repository at this point in the history

  2. Add current node to TC.is_met() method to access node properties (fac… 

    …ebook#2893)

Summary:

**Context:** There are some cases where the input constructor returns n=o, specifically for repeat arms, this is currently causing an issue in generation strategy gen method because if we don't generate from a node we can't meet the TC, and move forward, however, we don't actually want to generate from that node.

**This diff** updates tc.is_met() to accept a full generation node which allows us to access the should_skip property we added in the previous diff

Following diffs will:
- update input constructor to properly set should skip
- update the gs to appropiately handle resetting to false after a gen is completed

Reviewed By: lena-kashtelyan

Differential Revision: D64445871
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    7c4e023 View commit details
    Browse the repository at this point in the history

  3. Add should_skip setting to input constructors (facebook#2894) 

    Summary:




**Context:** There are some cases where the input constructor returns n=o, specifically for repeat arms, this is currently causing an issue in generation strategy gen method because if we don't generate from a node we can't meet the TC, and move forward, however, we don't actually want to generate from that node.

**This diff** adds the setting of should skip to repeat_arm_n input constructor as it's currently the only input constructor that could enter a condition that creates a skippable node

Following diffs will:
- update the gs to appropiately handle resetting to false after a gen is completed

Reviewed By: lena-kashtelyan

Differential Revision: D64475782
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    791b95f View commit details
    Browse the repository at this point in the history