Skip to content

Conversation

mlubin
Copy link
Member

@mlubin mlubin commented Feb 24, 2019

@codecov-io
Copy link

codecov-io commented Feb 24, 2019

Codecov Report

Merging #679 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #679   +/-   ##
=======================================
  Coverage   94.96%   94.96%           
=======================================
  Files          50       50           
  Lines        5347     5347           
=======================================
  Hits         5078     5078           
  Misses        269      269
Impacted Files Coverage Δ
src/attributes.jl 95.71% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bdae58...7731e59. Read the comment docs.

@mlubin
Copy link
Member Author

mlubin commented Feb 24, 2019

I'm looking at this again and I'm questioning why we have a VariableBasisStatus() attribute. Shouldn't this be the BasisStatus() on the single-variable constraints instead? As it is currently, this is the only place in MOI where "variable bounds" need to be discussed as a concept, which seems inconsistent.

@EdvinAblad
Copy link
Contributor

Consistency is important, however, I think that the most common use case is for a user to query whether a variable is basic or non-basic, not if a constraint is...

Is it possible (and sane) to have both?
E.g., calling MOI.get(m, MOI.BasisStatus(), var_index), is basically an indirect call to MOI.get(m, MOI.BasisStatus(), var_constr). (where var_constr is the applicable one)

Also I'm not quite sure how these single variable constraints works if they are called multiple times, e.g. x => 0, x >= 3, x =>2. In JuMP they seems to be available from set_lower_bound, in LQOI adding a single-variable bound also (seems to) replace the previous ones. Is it possible for multiple variable bounds to be present? If so, the basis status of the dominated constraints should always return MOI.BASIC (where should this domination check be done, LQOI?).

@blegat
Copy link
Member

blegat commented Feb 24, 2019

Is it possible for multiple variable bounds to be present?

No, see #570 The LowerBoundRef gives in JuMP gives you the reference of the last bound set

@mlubin mlubin changed the title clarify when NONBASIC_AT_* should be used Remove VariableBasisStatus and clarify when NONBASIC_AT_* should be used Feb 24, 2019
@mlubin
Copy link
Member Author

mlubin commented Feb 24, 2019

I updated the PR to remove VariableBasisStatus. It's not only inconsistent with how we report dual solutions, it's also redundant because you can ask for the ConstraintBasisStatus of a SingleVariable constraint anyway. Given that there's a direct mapping between the two, we shouldn't force solver interfaces to implement both. Convenience functions can be added at the JuMP level like we do for lower and upper bounds.

@odow @joaquimg

@mlubin mlubin merged commit 3cac190 into master Feb 25, 2019
@mlubin mlubin deleted the ml/basicdocs branch February 25, 2019 22:42
@odow odow mentioned this pull request Apr 16, 2021
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.

4 participants