Skip to content

Add additional inequality satisfaction convergence criterion#3240

Merged
jonmaddock merged 3 commits intomainfrom
3238-ensure-inequality-constraint-satisfaction-in-convergence-criteria
Jul 4, 2024
Merged

Add additional inequality satisfaction convergence criterion#3240
jonmaddock merged 3 commits intomainfrom
3238-ensure-inequality-constraint-satisfaction-in-convergence-criteria

Conversation

@jonmaddock
Copy link
Copy Markdown
Contributor

Ensure that inequality constraints are satisfied at the solution point.

This requires ukaea/PyVMCON#16 to be merged first, to actually call any additional convergence criterion.

This change will only affect optimising regression tests without f-values (currently only large_tokamak_nof.IN.DAT and stellarator.IN.DAT.) However, the large tokamak case already has satisfied inequality constraints, so it doesn't change. The stellarator test takes over 400 iterations running locally, so I haven't bothered running it. Before this is merged, I'll find current evidence of this actually changing the solution to having all-satisfied inequality constraints.

@jonmaddock jonmaddock requested a review from timothy-nunn July 2, 2024 14:06
@jonmaddock jonmaddock self-assigned this Jul 2, 2024
@jonmaddock jonmaddock linked an issue Jul 2, 2024 that may be closed by this pull request
@jonmaddock jonmaddock force-pushed the 3238-ensure-inequality-constraint-satisfaction-in-convergence-criteria branch from 1b81d62 to 28d1ac6 Compare July 3, 2024 15:11
@jonmaddock jonmaddock marked this pull request as ready for review July 3, 2024 20:14
@jonmaddock
Copy link
Copy Markdown
Contributor Author

After checking again on an updated main, the stellarator regression test did converge after 18 iterations, but it had 2 violated inequality constraints. With this additional convergence criterion, it now does not converge.

I haven't been able to find a current example where this requirement (satisfaction of inequality constraints) results in a different, fully satisfied, converging solution, although I have experienced this with the large tokamak regression test in the past. Currently the large tokamak regression test converges with satisfied inequality constraints without this additional criterion anyway.

I believe this should be merged anyway, as a solution isn't feasible without this criterion being met.

Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please make an issue regarding re-enabling Stellarator regression test.

@jonmaddock
Copy link
Copy Markdown
Contributor Author

Thanks Tim: added #3245 for the stellarator regression test re-introduction.

@jonmaddock jonmaddock merged commit 57625e3 into main Jul 4, 2024
@jonmaddock jonmaddock deleted the 3238-ensure-inequality-constraint-satisfaction-in-convergence-criteria branch July 4, 2024 10:07
timothy-nunn added a commit that referenced this pull request Jul 5, 2024
chris-ashe pushed a commit that referenced this pull request Jul 8, 2024
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.

Ensure inequality constraint satisfaction in convergence criteria

2 participants