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

Updates for MOI 0.9 #170

Merged
merged 7 commits into from
Aug 15, 2019
Merged

Updates for MOI 0.9 #170

merged 7 commits into from
Aug 15, 2019

Conversation

mlubin
Copy link
Member

@mlubin mlubin commented Jun 6, 2019

Do not merge until MOI 0.9 is tagged.

src/MOI_wrapper.jl Outdated Show resolved Hide resolved
@@ -12,7 +12,7 @@ MathProgBase = "fdba3010-5040-5b88-9595-932c9decdf73"

[compat]
BinaryProvider = "≥ 0.5.3"
MathOptInterface = "~0.8.1"
MathOptInterface = "0.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "~0.9"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

Both "0.9" and "~0.9" are equivalent, no ?: https://julialang.github.io/Pkg.jl/v1/compatibility/
For "0.9", it's ok if you don't change the left-most nonzero so the interval is [0.9, 0.10).
For "~0.9"~, it's ok if you only modify patch so the interval is [0.9, 0.10)` as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Caret specifiers are the default and hence 1.2.3 == ^1.2.3
PkgB = "^1.2" # [1.2.0, 2.0.0)

If I understand correctly, "0.9" would allow anything up to 1.0.

Copy link
Member

Choose a reason for hiding this comment

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

A caret specifier allows upgrade that would be compatible according to semver. An updated dependency is considered compatible if the new version does not modify the left-most non zero digit in the version specifier.

For "1.2.3", the left-most is "1" so "1.3" does not modify the left-most nonzero.
For "0.9", the left-most is "9" so "0.10" does modify the left-most nonzero.

@Wikunia
Copy link
Contributor

Wikunia commented Jul 10, 2019

According to jump-dev/MathOptInterface.jl#736
DualObjectiveValue should be implemented which isn't the case as it's not supported.
Therefore the test config is changed. Is this the way it should be handled or should it return nothing or something like that? The requirements read like the function should exist to support MOI v0.9

@blegat
Copy link
Member

blegat commented Jul 10, 2019

This was discussed on Gitter on June 03.
The guideline if it's not implemented by a solver is to use MOIU.get_fallback. In this case, to quote @mlubin in Gitter (June 03 13h31):

The bigger issue is that dual objective is not a well-defined concept for NLP solvers in general, as far as I'm aware. We can compute it manually if all constraints are linear, but that's about it.

So we could implement it and call MOIU.get_fallback if the problem is linear the throw an error if it's not.

@mlubin
Copy link
Member Author

mlubin commented Aug 14, 2019

I don't expect to have time to tidy this up and merge before the weekend, so it's up for grabs until then.

@blegat
Copy link
Member

blegat commented Aug 14, 2019

@mlubin Good to merge and release as Ipopt v0.6.1 ?

.travis.yml Outdated
@@ -11,6 +11,9 @@ sudo: false
addons:
apt_packages:
- gfortran
# TODO: Remove before merging.
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be removed

@@ -12,7 +12,7 @@ MathProgBase = "fdba3010-5040-5b88-9595-932c9decdf73"

[compat]
BinaryProvider = "≥ 0.5.3"
MathOptInterface = "~0.8.1"
MathOptInterface = "0.9"
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

@mlubin mlubin merged commit e53edd3 into master Aug 15, 2019
@mlubin mlubin deleted the ml/moi09 branch August 15, 2019 10:34
@ccoffrin
Copy link
Contributor

Thanks Miles!!!

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.

None yet

4 participants