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

maybe_add_self_consistency doesn't work for single-stage optimization #1234

Closed
ddudt opened this issue Aug 29, 2024 · 2 comments · Fixed by #1354
Closed

maybe_add_self_consistency doesn't work for single-stage optimization #1234

ddudt opened this issue Aug 29, 2024 · 2 comments · Fixed by #1354
Assignees
Labels
bug Something isn't working coil stuff relating to coils and coil optimization easy Short and simple to code or review objectives Adding or improving objective functions optimization Adding or improving optimization methods P3 Highest Priority, someone is/should be actively working on this

Comments

@ddudt
Copy link
Collaborator

ddudt commented Aug 29, 2024

The optimizer only calls maybe_add_self_consistency if the objective is not a ProximalProjection. This means it will not get called in single-stage optimization, and the corresponding curve constraints will not be applied.

A possible solution would be to split maybe_add_self_consistency into two separate functions: one for constraints applied to the Equilibrium and another for all other things. The former would still only be applied when the objective is not a ProximalProjection, but the latter would always get called for all optimizations. This is because the proximal optimizer has special logic specific to equilibria.

Obviously the user can manually add them for now to make this work.

@ddudt ddudt added bug Something isn't working objectives Adding or improving objective functions optimization Adding or improving optimization methods coil stuff relating to coils and coil optimization easy Short and simple to code or review labels Aug 29, 2024
@ddudt ddudt self-assigned this Aug 29, 2024
@ddudt ddudt changed the title maybe_add_self_consistency doesn't work forsingle-stage optimization maybe_add_self_consistency doesn't work for single-stage optimization Aug 29, 2024
@dpanici
Copy link
Collaborator

dpanici commented Nov 4, 2024

#1236 should take this into account

@dpanici
Copy link
Collaborator

dpanici commented Nov 4, 2024

if not isinstance(objective, ProximalProjection):

I think just swapping the if and the for loop here and adding a logical for if t is an Equilibrium should be enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working coil stuff relating to coils and coil optimization easy Short and simple to code or review objectives Adding or improving objective functions optimization Adding or improving optimization methods P3 Highest Priority, someone is/should be actively working on this
Projects
None yet
2 participants