Skip to content

Modified _parse_gdx_results in GAMS.py to replace _parse_special_value #3629

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

Closed
wants to merge 5 commits into from

Conversation

AnhTran01
Copy link

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_values with 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:

  1. I agree my contributions are submitted under the BSD license.
  2. 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.

@AnhTran01
Copy link
Author

@boxblox
@abhosekar

@boxblox
Copy link

boxblox commented Jun 16, 2025

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

Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

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

Note that the test infrastructure tests against a reasonably current GAMS on Jenkins and a very old version of GAMS on GHA, To get build feedback from GHA, I would suggest preserving compatibility withe old GAMS API.

Copy link

codecov bot commented Jun 17, 2025

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 ⚠️
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%) ⬇️
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.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jsiirola
Copy link
Member

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.

@AnhTran01
Copy link
Author

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?

@jsiirola
Copy link
Member

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.

@AnhTran01 AnhTran01 closed this Jun 23, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in July 2025 Release Jun 23, 2025
@AnhTran01 AnhTran01 deleted the ModifiedParseGDX branch June 23, 2025 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Updating the Import Statement to Use GAMS new API
4 participants