Skip to content

Conversation

blegat
Copy link
Member

@blegat blegat commented Jun 9, 2019

No description provided.

@codecov-io
Copy link

codecov-io commented Jun 9, 2019

Codecov Report

Merging #762 into master will not change coverage.
The diff coverage is 94.11%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #762   +/-   ##
=======================================
  Coverage   94.33%   94.33%           
=======================================
  Files          57       57           
  Lines        6226     6226           
=======================================
  Hits         5873     5873           
  Misses        353      353
Impacted Files Coverage Δ
src/Utilities/universalfallback.jl 97.54% <ø> (ø) ⬆️
src/Utilities/model.jl 91.46% <94.11%> (ø) ⬆️

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 057d027...82fb6d7. Read the comment docs.


## Models

The interface is generic and does not have a fixed list of functions and sets.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested text:

MOI is designed to be extensible, so there is no fixed list of possible functions and sets. This makes it challenging to define efficient storage representations for MOI models. For cases where the functions and sets of interest are known in advance (for example, solvers support a fixed list of functions and sets), we provide the Utilities.@model that macro defines a ModelLike given a list of functions and sets to support.

Utilities.UniversalFallback is a layer that sits on top of any ModelLike and provides non-specialized (slower) fallbacks for constraints and attributes that the underlying ModeLike does not support.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's better :)


"""
macro model(modelname, scalarsets, typedscalarsets, vectorsets, typedvectorsets, scalarfunctions, typedscalarfunctions, vectorfunctions, typedvectorfunctions)
macro model(modelname, scalarsets, typedscalarsets, vectorsets, typedvectorsets, scalarfunctions, typedscalarfunctions, vectorfunctions, typedvectorfunctions)
Copy link
Member

Choose a reason for hiding this comment

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

Can we provide a default instantiation of this type that supports all functions and sets that are defined in MOI? I think this would cover a lot of the existing use cases, and would save people from having to call the macro.

Copy link
Member Author

Choose a reason for hiding this comment

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

We already provide JuMP._MOIModel which I use most of the time

Copy link
Member

Choose a reason for hiding this comment

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

That's not a public symbol, and it requires depending on JuMP.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could define a @model(Model, ...) which is the same as JuMP's @model. Then JuMP won't have to define _MOIModel, it can just use MOI.Model, which decrease the chance of breaking JuMP with MOI.
I would be ok with that.

Copy link
Member

Choose a reason for hiding this comment

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

We could define a @model(Model, ...) which is the same as JuMP's @model. Then JuMP won't have to define _MOIModel, it can just use MOI.Model, which decrease the chance of breaking JuMP with MOI.
I would be ok with that.

I support this. I would call it something like DefaultModel or BasicModel to make it clear that it's possible to have different implementations of ModeLike. The @model macro then becomes a footnote, because most use cases are covered by BasicModel.

@blegat blegat merged commit 23a06a7 into master Jun 14, 2019
@odow odow deleted the bl/doc_uf_@model branch June 15, 2019 15:44
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.

3 participants