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

Update GenerationNode repr w/ brief node and model spec strings #3319

Closed
wants to merge 1 commit into from

Conversation

mgrange1998
Copy link
Contributor

Summary:

  1. Adjust GenNode.repr to only print TransitionCriterionClass(transition_to=<transition_to>) (can use tc.class.name for this), omitting other TransitionCriteria settings

  2. Adds a brief repr of ModelSpec repr to ignore its kwargs

  3. Reorders Generation Node args to node_name first, Model Specs after, then transition criteria last.

  4. Adds ' tick marks to "node_name" arg

  5. Update the corresponding unit tests

Note: Added a specific brief repr method to GenerationNode instead of changing TransitionCriteria's repr,
Additionally, added brief repr for model spec following prior feedback.

Differential Revision: D69260519

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 6, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69260519

mgrange1998 added a commit to mgrange1998/Ax that referenced this pull request Feb 6, 2025
…book#3319)

Summary:

1. Adjust GenNode.repr to only print `TransitionCriterionClass(transition_to=<transition_to>)` (can use tc.class.name for this), omitting other TransitionCriteria settings

2. Adds a brief repr of ModelSpec repr to ignore its kwargs

3. Reorders Generation Node args to node_name first, Model Specs after, then transition criteria last. 
4. Adds ' tick marks to "node_name" arg
5. Update the corresponding unit tests


Note: Added a specific brief repr method to GenerationNode instead of changing TransitionCriteria's repr, 
Additionally, added brief repr for model spec following prior feedback.

Differential Revision: D69260519
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69260519

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.70%. Comparing base (e8e1333) to head (6bcf910).

Files with missing lines Patch % Lines
ax/generation_strategy/generation_node.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3319      +/-   ##
==========================================
- Coverage   95.70%   95.70%   -0.01%     
==========================================
  Files         532      532              
  Lines       52450    52467      +17     
==========================================
+ Hits        50196    50212      +16     
- Misses       2254     2255       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…book#3319)

Summary:

1. Adjust GenNode.repr to only print `TransitionCriterionClass(transition_to=<transition_to>)` (can use tc.class.name for this), omitting other TransitionCriteria settings

2. Adds a brief repr of ModelSpec repr to ignore its kwargs

3. Reorders Generation Node args to node_name first, Model Specs after, then transition criteria last. 
4. Adds ' tick marks to "node_name" arg
5. Update the corresponding unit tests


Note: Added a specific brief repr method to GenerationNode instead of changing TransitionCriteria's repr, 
Additionally, added brief repr for model spec following prior feedback.

Reviewed By: mgarrard, lena-kashtelyan

Differential Revision: D69260519
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69260519

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 1a3715b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants