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

Update model-guide.rst #186

Merged
merged 15 commits into from
Sep 27, 2022
Merged

Update model-guide.rst #186

merged 15 commits into from
Sep 27, 2022

Conversation

4er4er4er
Copy link
Contributor

I am revising the modeling guide written by Gleb for MP-based solvers, based on updated categories that I developed for recent talks. I have already made some changes to the develop branch, but has become clear that continuing in that way would leave the guide potentially in a confusing state for a few weeks; thus I am working in a different branch from this point onward.

@4er4er4er 4er4er4er marked this pull request as draft August 31, 2022 21:43
@glebbelov
Copy link
Contributor

glebbelov commented Sep 1, 2022

Both commits on model-guide.rst look fine to me.

If possible, let's avoid long-lived branches. In any case, let's merge regularly both ways, earlier = better.

Copy link
Contributor

@glebbelov glebbelov left a comment

Choose a reason for hiding this comment

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

Both commits on model-guide.rst look fine to me. Note that the 1st commit, being on the develop branch, is already online at amplmp.readthedocs.io.

If possible, let's avoid long-lived branches. In any case, let's merge regularly both ways, earlier = better.

@4er4er4er
Copy link
Contributor Author

I am going to make some revisions now to the syntax terminology in the update branch (AMPL-MP-Modeling-Guide-Revised). This will temporarily cause some inconsistencies, so I won't make further commits until these revisions are done.

Copy link

@gomfy gomfy left a comment

Choose a reason for hiding this comment

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

  • Under Expressions supported section, the sentences regarding extensions:
    How about introducing some bullet points? For example,

    MP automatically enables extensions, which support:

    • conditional, logical, or counting operators,
    • model transformations between equivalent problem classes (e.g. quadratic to linear),
    • native solver extensions.

    Regarding extensions handled by AMPL, are those not handled by MP? If not, then I think it would be useful to mention that.

  • Under Expressions supported section/constr bullet point:
    There is an on missing after depending. The sentence should read:

    represents a constraint of the model, which may evaluate to true or false depending on the values of variables that it contains.

  • Under Expressions supported section, last sentence:
    How about rephrasing it like below?

    AMPL represents these as expression trees, which are sent to MP-based solver interfaces to be processed as required by the underlying solver.

@glebbelov glebbelov marked this pull request as ready for review September 27, 2022 00:06
@glebbelov glebbelov merged commit 6213e07 into develop Sep 27, 2022
@glebbelov glebbelov deleted the AMPL-MP-Modeling-Guide-Revised branch September 27, 2022 00:07
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.

3 participants