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

Support multiple node generation for single trial #2428

Closed
wants to merge 5 commits into from
Closed

Support multiple node generation for single trial #2428

wants to merge 5 commits into from

Commits on May 10, 2024

  1. Add AutoTransitionCriterion (facebook#2409) 

    Summary:

In order to support multiple models in a single gen call we need to support the `Auto` Transition class. This criterion will automatically move to the next node once anything has been generated from the current node

Reviewed By: saitcakmak

Differential Revision: D56360662
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed May 10, 2024
    Configuration menu
    Copy the full SHA
    862fa50 View commit details
    Browse the repository at this point in the history

  2. Improve test_generation_strategy file 

    Summary: While I was working on adding some new tests I noticed we could be using the vars defined in setup much more in the file to reduce redundancy

Reviewed By: lena-kashtelyan

Differential Revision: D56744357
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed May 10, 2024
    Configuration menu
    Copy the full SHA
    195f923 View commit details
    Browse the repository at this point in the history

  3. model <> node 2/n: Add transition edges property to GenerationNode (f… 

    …acebook#2446)

Summary:

As part of modifying the generation node dag we want to support multiple transition edges now (meaning one node x could transition either to node y or node z depending on what criterion are met). This property creates a dict with {'next_node_name': [tc]} where tc is the transition criterion to move the gs from the current node to the `next node`

The reason we need this, is currently and in any future i can envision, to transition to another node all criterion for that edge that are transition blocking must be met. In the next diff I modify the should_transition method. I just like bite sized diffs :)

Reviewed By: lena-kashtelyan

Differential Revision: D57127974
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed May 10, 2024
    Configuration menu
    Copy the full SHA
    4c91f12 View commit details
    Browse the repository at this point in the history

  4. node <> model 3/n: update should_transition_to_next_node (facebook#2447) 

    Summary:

This diff builds on the previous diff, and uses the new property that we added for TC to decide if the gs should move forward.

If all criterion that specificy a transition edge between nodes are met, we move to that node. We progress through tc in order -- meaning when constructing a gs tc ordering is important.

Differential Revision: D57132018
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed May 10, 2024
    Configuration menu
    Copy the full SHA
    ec742c9 View commit details
    Browse the repository at this point in the history

  5. model <> node 4/n: Support multiple node generation for single trial (f… 

    …acebook#2428)

Summary:

This diff enables multiple nodes to be used to generate a single batch trial. Right now the limitations are that: 
(1) currently each node only contributes 1 gr to the trial -- we will extend this to n grs in a future diff 

To do this we added:
(1) should_move_trials method
(2) updates to the transition criterion args

Differential Revision: D56743651
    @mgarrard @facebook-github-bot
    mgarrard authored and facebook-github-bot committed May 10, 2024
    Configuration menu
    Copy the full SHA
    0cabf69 View commit details
    Browse the repository at this point in the history