Skip to content

Conversation

@lena-kashtelyan
Copy link
Contributor

Summary:

A lot going on here:

  1. Make GenerationStep a factory for GenerationNode by replacing its __init__(self, ...) constructor (which would have to return a GStep) with a __new__(cls, ...) construtor, which can return a GNode (magic, thanks Devmate!)
  2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
  3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: mgarrard

Differential Revision: D80128678

@meta-cla meta-cla bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jan 7, 2026
@meta-codesync
Copy link

meta-codesync bot commented Jan 7, 2026

@lena-kashtelyan has exported this pull request. If you are a Meta employee, you can view the originating Diff in D80128678.

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 98.05353% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.71%. Comparing base (7bce65b) to head (ad820b9).

Files with missing lines Patch % Lines
ax/generation_strategy/generation_node.py 95.12% 2 Missing ⚠️
ax/storage/sqa_store/decoder.py 81.81% 2 Missing ⚠️
ax/generation_strategy/generation_strategy.py 97.50% 1 Missing ⚠️
ax/storage/json_store/decoder.py 66.66% 1 Missing ⚠️
ax/storage/json_store/encoders.py 50.00% 1 Missing ⚠️
ax/utils/testing/modeling_stubs.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4746      +/-   ##
==========================================
- Coverage   96.72%   96.71%   -0.01%     
==========================================
  Files         583      583              
  Lines       60860    60807      -53     
==========================================
- Hits        58866    58811      -55     
- Misses       1994     1996       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lena-kashtelyan pushed a commit to lena-kashtelyan/Ax that referenced this pull request Jan 11, 2026
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: mgarrard

Differential Revision: D80128678
lena-kashtelyan pushed a commit to lena-kashtelyan/Ax that referenced this pull request Jan 11, 2026
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: mgarrard

Differential Revision: D80128678
@lena-kashtelyan lena-kashtelyan force-pushed the export-D80128678 branch 2 times, most recently from bb20f90 to 956f26b Compare January 11, 2026 17:31
lena-kashtelyan pushed a commit to lena-kashtelyan/Ax that referenced this pull request Jan 11, 2026
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: mgarrard

Differential Revision: D80128678
lena-kashtelyan pushed a commit to lena-kashtelyan/Ax that referenced this pull request Jan 12, 2026
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: mgarrard

Differential Revision: D80128678
@lena-kashtelyan lena-kashtelyan force-pushed the export-D80128678 branch 2 times, most recently from d9ffcef to 6f0f076 Compare January 14, 2026 19:03
lena-kashtelyan pushed a commit to lena-kashtelyan/Ax that referenced this pull request Jan 14, 2026
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Differential Revision: D80128678
Summary:
X-link: facebookincubator/MCGrad#94

Pull Request resolved: facebook#4746

Pull Request resolved: facebook#4731

A lot going on here:

1. Make `GenerationStep` a factory for `GenerationNode` by replacing its `__init__(self, ...)` constructor (which would have to return a `GStep`) with a `__new__(cls, ...)` construtor, which can return a `GNode` (magic, thanks Devmate!)
2. Adapt storage: stop storing steps and just treat them as nodes. No step-only fields will be saved going forward. Backward compatibility is handled though.
3. Change a bazillion tests and checks in downstream applications.

NOTE: need to remember next steps here: https://www.internalfb.com/diff/D86066476?dst_version_fbid=732904879835890&transaction_fbid=1400151778204976, cc mgarrard

sorrybigdiff

Reviewed By: lena-kashtelyan

Differential Revision: D80128678
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 meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants