-
Notifications
You must be signed in to change notification settings - Fork 3
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
Enrich README with explanation of AMPL code #31
Conversation
Signed-off-by: parvy <pierre.arvy@artelys.com>
Kudos, SonarCloud Quality Gate passed! |
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the huge amount of documentation work done here. I left minor comments.
Regarding Artelys advertisement about selling keys, is it appropriate to include it in here though? I am not sure of our policy on that matter but it startled me a little :)
open-reac/README.md
Outdated
are free to configure a different one. | ||
|
||
If you chose to run Knitro, you must have `knitroampl` in your path, after the installation | ||
of the solver is done and that you got a valid licence. To check, start a bash and run: | ||
```bash | ||
knitroampl stub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this command line suppose to produce? What I've got is the following:
##### This license is only [...]. #####
##### License is valid until [...] #####
knitroampl: can't open stub.nl
It would be nice to give the output so that a beginner may know what to expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment, as well as those indicating that Artelys sells keys for AMPL and Knitro were already in the README and come from this commit #1 .
I also get this result running the command. Despite the error, this indicates that the knitroampl command is recognised. But it could be confusing for the user indeed.
I propose to remove these comments. What do you think @So-Fras ?
Signed-off-by: parvy <pierre.arvy@artelys.com>
Signed-off-by: parvy <pierre.arvy@artelys.com>
Quality Gate passedIssues Measures |
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
Add user documentation of AMPL code in README
What is the current behavior?
No documentation is available (except some comments in AMPL code)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API?
Other information: