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

Fix hs61 test #334

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Fix hs61 test #334

merged 1 commit into from
Jun 21, 2024

Conversation

tmigot
Copy link
Member

@tmigot tmigot commented Jun 21, 2024

@amontoison
Copy link
Member

I already updated hs61 a few weeks ago.
f01b291

@amontoison amontoison marked this pull request as ready for review June 21, 2024 15:14
@amontoison amontoison merged commit c9820df into main Jun 21, 2024
14 of 17 checks passed
@amontoison amontoison deleted the fix-quad branch June 21, 2024 15:15
@tmigot
Copy link
Member Author

tmigot commented Jun 21, 2024

Ok, but the issue remains because we also need to replace @NLconstraint by @constraint ...

@amontoison
Copy link
Member

We need to do that only if we want to switch to the new API. It's related to the PR #321.

@tmigot
Copy link
Member Author

tmigot commented Jun 21, 2024

Yes and no, using @constraint for quadratic constraint is possibly a better modeling, the fact that use @NLconstraint was a hack. I don't think this is particularly connected to the new API.
Thanks to the great work in NLPModelsJuMP we will be able to update quadratic constraints, that was the point of this. Also this PR was a draft, there was no need to merge it.

@amontoison
Copy link
Member

@NLconstraint was mandatory for quadratic constraints before MOI, it was not a hack.
JuMP just added support for it later and us now.

The new API is now doing the same thing but for all nonlinear constraints /objective.

I merged the PR becasue we can test hs61 in all cases now, even with the old API.

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.

Fix unit tests for hs61
2 participants