Skip to content

[Utilities] fix use of recursion in canonicalize!(::ScalarNonlinearFunction)#3009

Merged
odow merged 1 commit into
masterfrom
od/canonical
Jun 11, 2026
Merged

[Utilities] fix use of recursion in canonicalize!(::ScalarNonlinearFunction)#3009
odow merged 1 commit into
masterfrom
od/canonical

Conversation

@odow

@odow odow commented Jun 9, 2026

Copy link
Copy Markdown
Member

Alternative to #3008

@blegat

blegat commented Jun 9, 2026

Copy link
Copy Markdown
Member

Why is it an alternative ? Do you preserve aliases ?

@odow

odow commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

Your one doesn't do any. This PR fixes the use of recursion. There's an open question about aliases etc.

I think this PR would automatically account for aliases because if it reaches duplicate ScalarAffineFunction that need canonicalisation, then the first will update in place and we won't need to check the second.

@blegat

blegat commented Jun 9, 2026

Copy link
Copy Markdown
Member

Good point, canonical! is alias-friendly. I opened #3008 just to make that branch visible as it was needed in jump-dev/JuMP.jl#4032 but it's indeed not implementing the alias-preservation at all yet. I think this PR is an improvement nevertheless so we can merge it

@odow

odow commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I'll fix the formatting in a separate PR.

@odow odow merged commit 4d85a2a into master Jun 11, 2026
29 of 30 checks passed
@odow odow deleted the od/canonical branch June 11, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants