Modified _parse_gdx_results in GAMS.py to replace _parse_special_value
Fixes #3624
Summary/Motivation:
When parsing the results of a model after it has been solved, the level and dual value are obtained through a series of if statements in _parse_special_values that may cause slowdowns. This PR added GAMS existing functions to handle data parser for these special values in _parse_gdx_results.
Changes proposed in this PR:
- Replaced
_parse_special_valueswith GAMS special value parser in_parse_gdx_results.
Legal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:
- I agree my contributions are submitted under the BSD license.
- I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.
@boxblox @abhosekar
this is a small thing, but we should add gdx.gdxLibraryUnload() after the gdxFree(pgdx) calls in both the results file and the stats file read operations
Codecov Report
Attention: Patch coverage is 86.17021% with 13 lines in your changes missing coverage. Please review.
Project coverage is 85.75%. Comparing base (
ca33341) to head (e1da345).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| pyomo/solvers/plugins/solvers/GAMS.py | 86.17% | 13 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #3629 +/- ##
==========================================
- Coverage 88.93% 85.75% -3.19%
==========================================
Files 888 888
Lines 102406 102435 +29
==========================================
- Hits 91079 87844 -3235
- Misses 11327 14591 +3264
| Flag | Coverage Δ | |
|---|---|---|
| builders | 26.68% <15.95%> (-0.01%) |
:arrow_down: |
| default | 85.58% <86.17%> (?) |
|
| expensive | 34.04% <15.95%> (?) |
|
| linux | ? |
|
| linux_other | ? |
|
| osx | ? |
|
| win | ? |
|
| win_other | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
It is unfortunate that e1da345 was committed... that introduced a bunch of non-functional changes that are confusing review of this PR. For the future, we run (and test against) black -C -S. If you want to make all the changes to the string quote characters, that's fine, but it would be better for that to be isolated to a separate PR that ONLY makes that change.
This is my bad @jsiirola. In this case, would the best practice is to make a new PR on a clean branch with the changes to the import statement?
This is my bad @jsiirola. In this case, would the best practice is to make a new PR on a clean branch with the changes to the import statement?
That is completely up to you. You can just revert that commit on this branch and it is likely to be OK. If that still leaves "a mess" then you can decide if it is easier to just recreate a new branch / PR.