updating the file to use cplex correctly #64
Conversation
|
Thanks for the PR @ceserz2. Typically, a pull request is made to resolve a particular issue and includes a description of the changes that were made to achieve this fix. If there is an issue or bug, please create a descriptive “issue” explaining the issue or bug, what steps you took before the issue occurred, and what steps you took to try resolving it. Along with other information like the version and platform you’re using. Please see the |
pulling from main into my forked repo
There was a problem hiding this comment.
Hi @ceserz2, this PR is still missing some information!
What issues are associated with this PR? How do you know this is the "correct" string for CPLEX? Are there other places for the documentation to be improved as well? Perhaps you could add guidance for installing CPLEX on macos?
A couple of general tips:
- It's not a good idea to change default behavior based on an operating system you haven't tested your code on. E.g., the docs specify installing the optional dependencies with
pip install -e .[doc]for a windows or unix machine, but on macos you have to dopip install -e .'[doc]'-- which may not work on the other two operating systems. Make sense? - Reviewers and assignees are not the same entity! An assignee is typically the person(s) responsible for working with the maintainers to adjust their PRs. A reviewer is typically one of the maintainers that have been asked to review a pull request. In this case, I should be a reviewer rather than an assignee.
| @@ -54,9 +54,11 @@ | |||
| "\n", | |||
| "# set the solver based on operating system -- assumes glpk or cbc is installed.\n", | |||
| "if \"win32\" in sys.platform:\n", | |||
| " solver = 'cplex'\n", | |||
| " solver = 'cplex_direct'\n", | |||
There was a problem hiding this comment.
How do you know this line needs to be changed? Have you tested this on a windows computer? (I have and it seems to work fine).
| "elif \"darwin\" in sys.platform:\n", | ||
| " solver = \"cplex_direct\"\n", |
There was a problem hiding this comment.
Do you know this will always be necessary or does it depend on how you installed CPLEX?
|
Thanks for the PR, @ceserz2! I'm going to close this PR since I don't think cplex needs to be an available option in the example notebooks. However, I would welcome a future PR with a docs page that guides users to install CPLEX on their computers. |
This PR updates the capacity expansion tutorial to incorporate the correct usage of cplex. This PR changes how cplex is called in the file from 'cplex' to 'cplex_direct'