RFC: more docs #1436

odow · 2018-08-23T04:48:01Z

Continuation of my doc-writing quest in order to learn the new JuMP syntax.

The majority of the quickstart is borrowed from https://github.com/JuliaOpt/JuMP.jl/blob/v0.18.2/doc/example.rst

~~The most contentious change is that I added GLPK to travis so that we have a solver for the doc tests.~~ We now use a mock optimizer.

codecov · 2018-08-23T05:30:29Z

Codecov Report

Merging #1436 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1436      +/-   ##
==========================================
+ Coverage   89.22%   89.28%   +0.05%     
==========================================
  Files          26       26              
  Lines        3723     3723              
==========================================
+ Hits         3322     3324       +2     
+ Misses        401      399       -2

Impacted Files	Coverage Δ
src/parseexpr.jl	`94.88% <0%> (+0.78%)`	⬆️

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 4496a54...3598a59. Read the comment docs.

blegat · 2018-08-23T07:55:00Z

The output of the doctests on Travis seem to indicate failures:

=====[Test Error]==============================

Location: src/quickstart.md:63-65
Code block:

julia> con = @constraint(model, 1x + 5y <= 3)
x + 5 y <= 3.0

Subexpression:
con = @constraint(model, 1x + 5y <= 3)
Output Diff (REQUIRES COLOR):
x + 5 y <= ≤ 3.0
=====[End Error]===============================
=====[Test Error]==============================
Location: src/quickstart.md:128-139
Code block:

julia> x_upper = JuMP.UpperBoundRef(x)
x <= 2.0
julia> JuMP.resultdual(x_upper)
-4.4
julia> y_lower = JuMP.LowerBoundRef(y)
y >= 0.0
julia> JuMP.resultdual(y_lower)
0.0

Subexpression:
x_upper = JuMP.UpperBoundRef(x)
Output Diff (REQUIRES COLOR):
x <= ≤ 2.0
=====[End Error]===============================
=====[Test Error]==============================
Location: src/quickstart.md:128-139
Code block:

julia> x_upper = JuMP.UpperBoundRef(x)
x <= 2.0
julia> JuMP.resultdual(x_upper)
-4.4
julia> y_lower = JuMP.LowerBoundRef(y)
y >= 0.0
julia> JuMP.resultdual(y_lower)
0.0

Subexpression:
y_lower = JuMP.LowerBoundRef(y)
Output Diff (REQUIRES COLOR):
y >= ≥ 0.0
=====[End Error]===============================

Wouldn't it be preferable to put the using GLPK outside the jldoctest and build a MockOptimizer instead in setup and feed it with the expected outputs ?

odow · 2018-08-23T08:39:02Z

Urgh, this is some platform issue. For me, the docs build locally on v0.6 without errors.

blegat · 2018-08-25T07:02:53Z

docs/src/variables.md

@@ -130,9 +135,9 @@ In the above examples, `x_free` represents an unbounded optimization variable,
    `@variable(model, a <= x)`) will result in an error.


a=1 above should be a = 1 according to the style guide

blegat · 2018-08-25T07:11:03Z

docs/src/quickstart.md

+familiar with another modeling language embedded in a high-level language such
+as PuLP (Python) or a solver-specific interface you will find most of this
+familiar, with the exception of *macros*. A deep understanding of macros is not
+essential, but if you would like to know more please see the


Reading this, it seems to me that it would be a good idea to first understanding macros while this really not a good idea to learn metaprogramming to use JuMP. This might discourage many users. I don't know if essential is the weak enough, maybe "needed" or "required" ? Not sure :)

Agreed that pointer to macros is a bit out of place. The quick-start guide should be a gentle introduction that shows how to get a few basic things done using JuMP.

blegat · 2018-08-25T07:12:36Z

docs/src/quickstart.md

+```
+!!! note
+    Your model doesn't have to be called `model` - it's just a name. However,
+    the JuMP style guide prefers `model`.


Maybe add add href to the style guide here

The second sentence is maybe too much information. The style guide doesn't have an explicit preference for model, just a suggestion to avoid one-letter variable names.

blegat · 2018-08-25T07:12:46Z

docs/src/quickstart.md

+DocTestFilters = r"JuMP\."
+```
+!!! note
+    Your model doesn't have to be called `model` - it's just a name. However,


doesn't -> does not

Not sure if we want to allow shortening or not in fact. If you think it is more friendly then I am ok to leave it :)

blegat · 2018-08-25T07:13:52Z

docs/src/quickstart.md

+DocTestSetup = nothing
+```
+
+Next we'll set our objective. Note again the `model`, so we know which model's


we'll -> we will

So I started reading the Google documentation style guide. They recommend using contractions: https://developers.google.com/style/contractions

Ok let's follow Google's style guideline for contraction

mlubin · 2018-08-26T18:38:39Z

docs/src/quickstart.md

+familiar with another modeling language embedded in a high-level language such
+as PuLP (Python) or a solver-specific interface you will find most of this
+familiar, with the exception of *macros*. A deep understanding of macros is not
+essential, but if you would like to know more please see the


Agreed that pointer to macros is a bit out of place. The quick-start guide should be a gentle introduction that shows how to get a few basic things done using JuMP.

mlubin · 2018-08-26T18:38:55Z

docs/src/quickstart.md

+as PuLP (Python) or a solver-specific interface you will find most of this
+familiar, with the exception of *macros*. A deep understanding of macros is not
+essential, but if you would like to know more please see the
+[Julia documentation](http://docs.julialang.org/en/latest/manual/metaprogramming/).


Nit: Link to versioned docs.

mlubin · 2018-08-26T18:41:05Z

docs/src/quickstart.md

+```
+!!! note
+    Your model doesn't have to be called `model` - it's just a name. However,
+    the JuMP style guide prefers `model`.


The second sentence is maybe too much information. The style guide doesn't have an explicit preference for model, just a suggestion to avoid one-letter variable names.

mlubin · 2018-08-26T18:42:04Z

docs/src/quickstart.md

+it! Feel free to stick with `*` if it makes you feel more comfortable, as we
+have done with `3*y`:
+```jldoctest quickstart_example
+julia> @objective(model, Max, 5x + 3*y)


The style guide recommends spaces around binary operators, so 3 * y.

mlubin · 2018-08-26T18:43:16Z

docs/src/quickstart.md

+```
+In this case, `GLPK` returned `Success`. This does not mean that it has found
+the optimal solution. Instead, it indicates that GLPK has finished running and
+did not encounter any errors or termination limits.


Nit: user-provided termination limits

mlubin · 2018-08-26T18:51:46Z

docs/src/variables.md

@@ -130,9 +135,9 @@ In the above examples, `x_free` represents an unbounded optimization variable,
    `@variable(model, a <= x)`) will result in an error.

    **Extra for experts:** the reason for this is that at compile time, JuMP


This whole note is maybe too much information. I don't remember anyone ever being confused by why @variable(model, a <= x) doesn't work. The error message should be helpful enough to figure it out if it ever happens.

mlubin · 2018-08-26T19:00:39Z

docs/src/variables.md

+MathOptInterface
+```
+
+To delete variables, we can use the `MOI.delete!` method. We can also check


Let's implement JuMP.delete! and JuMP.is_valid instead of documenting the MOI way.

odow · 2018-09-01T06:37:37Z

We're good to go on my end.

Doctests pass on v0.6. Annoyingly, they fail on v0.7 because it does not print the module that defines the type. For example, v0.6 will print JuMP.VariableRef whereas v0.7 prints VariableRef. However, Documenter only has the ability to filter output that appears in both the docs and actual output.

mlubin · 2018-09-01T14:18:05Z

Could we run the doctests on only one version of Julia? Might as well switch to 1.0 unless there's something blocking it.

…

On Sat, Sep 1, 2018, 02:37 Oscar Dowson ***@***.***> wrote: Doctests pass on v0.6. Annoyingly, they fail on v0.7 because it does not print the module that defines the type. For example, v0.6 will print JuMP.VariableRef whereas v0.7 prints VariableRef. However, Documenter only has the ability to filter output that appears in both the docs and actual output. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1436 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABp0M2oYbPCzgzI2IAt8KtgqbQrSZYf6ks5uWisxgaJpZM4WI1so> .

odow · 2018-09-02T06:25:17Z

docs/src/variables.md


 julia> for key in [:A, :B]
-           variables[key] = @variable(model, [1:2, 1:2], Symmetric)
+           global variables[key] = @variable(model, [1:2, 1:2])


I still can't believe that this global is necessary.

odow · 2018-09-02T21:21:54Z

Doctests now run only on v1.0 and pass.

mlubin

Sorry for the slow review... was on vacation.

mlubin · 2018-09-05T22:33:30Z

docs/src/quickstart.md

+it! Feel free to stick with `*` if it makes you feel more comfortable, as we
+have done with `3 * y`:
+```jldoctest quickstart_example
+julia> @objective(model, Max, 5x + 3 * y)


Maybe point out that this example is intentionally inconsistent, but it's good practice to use one way or the other consistently in your code.

mlubin · 2018-09-05T22:34:51Z

docs/src/quickstart.md

+less-than-or-equal-to constraint using `<=`, but we can also create equality
+constraints using `==` and greater-than-or-equal-to constraints with `>=`:
+```jldoctest quickstart_example; filter=r"≤|<="
+julia> con = @constraint(model, 1x + 5y <= 3)


Why use the anonymous constraint syntax here and the named variable syntax above? (I'd recommend using the named constraint syntax for consistency.)

mlubin · 2018-09-05T22:37:41Z

docs/src/quickstart.md

+julia> JuMP.termination_status(model)
+Success::TerminationStatusCode = 0
+```
+In this case, `GLPK` returned `Success`. This does not mean that it has found


Emphasis on not

mlubin · 2018-09-05T22:39:00Z

docs/src/variables.md

@@ -44,6 +44,10 @@ This code does three things:
 To reduce confusion, we will attempt, where possible, to always refer to
 variables with their corresponding prefix.

+!!! warn
+    If you create two JuMP variables with the same name, an error will be


More directly: "Creating two JuMP variables with the same name results in an error at runtime."

mlubin · 2018-09-05T22:39:57Z

docs/src/variables.md

-Because `y` is a Julia variable, I can bind it to a different value. For example,
-if I go:
+Because `y` is a Julia variable, we can bind it to a different value. For
+example, if we go:


"if we go" sounds a bit too informal and might be confusing for non-native speakers. "if we write" instead?

odow · 2018-09-06T06:42:34Z

Sorry for the slow review... was on vacation.

We're not paying you enough to need to apologize...

blegat reviewed Aug 25, 2018

View reviewed changes

mlubin reviewed Aug 26, 2018

View reviewed changes

odow mentioned this pull request Aug 27, 2018

Expose JuMP.delete and JuMP.is_valid methods #1445

Merged

blegat approved these changes Aug 27, 2018

View reviewed changes

odow force-pushed the od/more-docs branch from 1363a8d to 8409211 Compare August 29, 2018 22:40

odow changed the title ~~WIP: more docs~~ RFC: more docs Sep 1, 2018

odow commented Sep 2, 2018

View reviewed changes

odow added 10 commits September 2, 2018 19:43

Discuss deleting variables and variable bounds

5911d9c

Add quickstart documentation

176170f

Remove GLPK dependency

beab8d1

Filter comparison operators

c9f7d6d

Filter our JuMP. and fix minor typos

d0fed02

Remove macro reference and other small style fixes

5825e8f

Update for JuMP renaming

d67ca11

Remove doctest filters

fa038e1

Move documenter to run on v1.0

1dac212

Rebase for MOI v0.6

897d32e

odow force-pushed the od/more-docs branch from c1866e7 to 897d32e Compare September 2, 2018 07:54

Fix a mistake from rebasing

2eb8a46

mlubin reviewed Sep 5, 2018

View reviewed changes

Updates for Miles

3598a59

mlubin merged commit a2f1285 into master Sep 6, 2018

mlubin deleted the od/more-docs branch September 6, 2018 12:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: more docs #1436

RFC: more docs #1436

odow commented Aug 23, 2018 •

edited

Loading

codecov bot commented Aug 23, 2018 •

edited

Loading

blegat commented Aug 23, 2018 •

edited

Loading

odow commented Aug 23, 2018

blegat Aug 25, 2018

blegat Aug 25, 2018

mlubin Aug 26, 2018

blegat Aug 25, 2018

mlubin Aug 26, 2018

blegat Aug 25, 2018

blegat Aug 25, 2018

blegat Aug 25, 2018

odow Aug 25, 2018

blegat Aug 25, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

mlubin Aug 26, 2018

odow commented Sep 1, 2018 •

edited

Loading

mlubin commented Sep 1, 2018 via email

odow Sep 2, 2018

odow commented Sep 2, 2018

mlubin left a comment

mlubin Sep 5, 2018

mlubin Sep 5, 2018

mlubin Sep 5, 2018

mlubin Sep 5, 2018

mlubin Sep 5, 2018

odow commented Sep 6, 2018

		@@ -130,9 +135,9 @@ In the above examples, `x_free` represents an unbounded optimization variable,
		`@variable(model, a <= x)`) will result in an error.

		@@ -130,9 +135,9 @@ In the above examples, `x_free` represents an unbounded optimization variable,
		`@variable(model, a <= x)`) will result in an error.

		Extra for experts: the reason for this is that at compile time, JuMP

RFC: more docs #1436

RFC: more docs #1436

Conversation

odow commented Aug 23, 2018 • edited Loading

codecov bot commented Aug 23, 2018 • edited Loading

Codecov Report

blegat commented Aug 23, 2018 • edited Loading

odow commented Aug 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odow commented Sep 1, 2018 • edited Loading

mlubin commented Sep 1, 2018 via email

Choose a reason for hiding this comment

odow commented Sep 2, 2018

mlubin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odow commented Sep 6, 2018

odow commented Aug 23, 2018 •

edited

Loading

codecov bot commented Aug 23, 2018 •

edited

Loading

blegat commented Aug 23, 2018 •

edited

Loading

odow commented Sep 1, 2018 •

edited

Loading