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

rework docs: simpler readme/index, modeling file as goto place #349

Merged
merged 15 commits into from
Jun 26, 2023
Merged

Conversation

tias
Copy link
Collaborator

@tias tias commented Jun 13, 2023

merged different versions of 'usage' docs into the file 'modeling'

then reworked it substantially to be more extensive, to be oriented to starting users, and to Python users specifically, so little to no CP knowledge.

It raised a range of other questions/loose ends, some of them indicated in the text (to be removed), some of them half-implemented. All code changes in this PR will be spun out in separate pull requests.

I tried to be succinct yet introduce cases people should know about in modeling.md. Helene @363734 is this in line with what you were looking for? Anything you feel missing or should be clarified?

@tias tias requested a review from 363734 June 13, 2023 14:47
Copy link
Collaborator

@IgnaceBleukx IgnaceBleukx left a comment

Choose a reason for hiding this comment

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

Really like the new docs, it reads super smooth and touches upon all the major parts.
Left some suggestions for re-ordering, streamlining or typos :)

cpmpy/model.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@@ -21,7 +21,7 @@
==================

A decision variable is a variable whose value will be determined by the solver.

h
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

probably a typo

cpmpy/model.py Outdated
if len(con) == 1:
con = con[0]

else:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can also be an elif but this is more readable

docs/modeling.md Outdated
```python
import cpmpy as cp
```
which we will use below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

which is how we will use it?

tests/test_expressions.py Outdated Show resolved Hide resolved
docs/modeling.md Outdated

`solution_hint()` tells the solver that it could use these variable-values first during search, e.g. typically from a previous solution:
```python
from cpmpy import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

use import cpmpy as cp?

docs/modeling.md Outdated
You provide it with the name of the function you want to call, as well as the arguments:

```python
from cpmpy import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

use import cpmpy as cp?

docs/modeling.md Outdated
Note that any argument given will be checked for whether it needs to be mapped to a native solver variable. This may give errors on complex arguments, or be inefficient. You can tell the `DirectConstraint` not to scan for variables with `noarg` argument, for example:

```python
from cpmpy import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

use import cpmpy as cp?

docs/modeling.md Outdated
The `DirectConstraint("AddAllDifferent", iv)` is equivalent to the following code, which demonstrates that you can mix the use of CPMpy with calling the underlying solver directly:

```python
from cpmpy import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

use import cpmpy as cp?

@363734
Copy link
Contributor

363734 commented Jun 15, 2023

@tias yes, this is better to avoid confusion.

Copy link
Contributor

@363734 363734 left a comment

Choose a reason for hiding this comment

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

sgtm! (execpt the typo)

@JoD JoD added the next release Needed for next release label Jun 19, 2023
@tias
Copy link
Collaborator Author

tias commented Jun 21, 2023

pulled out the code changes, all other feedback should be in. I will merge this somewhere tomorrow, can still be improved afterwards

@tias tias merged commit fd5663d into master Jun 26, 2023
@tias tias deleted the doc_rework branch June 26, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Needed for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants