Skip to content

Make problem as part of the state of ParamSpaceSGD instead of an argument of estimate_gradient! #185

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

Merged
merged 1 commit into from
Jul 30, 2025

Conversation

Red-Portal
Copy link
Member

@Red-Portal Red-Portal commented Jul 30, 2025

This changes how ParamSpaceSGD interacts with the target problem. Instead of receiving the problem prob as an argument of estimate_gradient!, it now stores it in the state when calling init. The main advantage of this choice is that, whenever some modifications/preparations need to be done to prob, it only needs to be done once in init. This is necessary for simplifying #180 .

@Red-Portal Red-Portal requested review from yebai and penelopeysm July 30, 2025 19:41
@Red-Portal Red-Portal changed the title Move problem as part of the state instead of an argument of estimate_gradient! Make problem as part of the state instead of an argument of estimate_gradient! Jul 30, 2025
@Red-Portal Red-Portal changed the title Make problem as part of the state instead of an argument of estimate_gradient! Make problem as part of the state of ParamSpaceSGD instead of an argument of estimate_gradient! Jul 30, 2025
@Red-Portal Red-Portal requested a review from sunxd3 July 30, 2025 20:42
@yebai
Copy link
Member

yebai commented Jul 30, 2025

I'm not sure why this simplifies #180, but it seems like a minor and harmless change, so I'm happy to approve.

@Red-Portal
Copy link
Member Author

Red-Portal commented Jul 30, 2025

Thanks, @yebai It's actually a split of #180

@Red-Portal Red-Portal force-pushed the problem_part_of_state branch from 8692bc6 to 20eeb6b Compare July 30, 2025 21:56
@Red-Portal Red-Portal merged commit ad5bf6e into TuringLang:main Jul 30, 2025
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants