Skip to content

Ignore errors on ASL solver version check#3298

Merged
mrmundt merged 4 commits intoPyomo:mainfrom
alma-walmsley:main
Jul 10, 2024
Merged

Ignore errors on ASL solver version check#3298
mrmundt merged 4 commits intoPyomo:mainfrom
alma-walmsley:main

Conversation

@alma-walmsley
Copy link
Copy Markdown
Contributor

Fixes # .

Summary/Motivation:

Trying to run the couenne solver executable through Pyomo throws an error. Pyomo tries to check the version of the executable specified to verify that the solver can be run. In couenne's case, running couenne -v returns couenne (Microsoft cl 15.00.21022.08 für 80x86), ASL(20100130), except the ü runs into a problem with default encoding / utf-8 and cannot be decoded. This throws an error in the subprocess call which is not caught and so the solver cannot be run. Note that this problem is not necessarily couenne specific, but rather due to subprocess expecting the data in a text format where special characters like ü could not be recognised.

To reproduce, for example:

from pyomo.environ import *

model = ConcreteModel()

model.x = Var(initialize=0, bounds=(0, 10))
model.obj = Objective(expr=model.x, sense=maximize)  # maximize x -> x=10

# solve the model
solver = SolverFactory("couenne")
solver.solve(model)  # throws an error: 'utf-8' codec can't decode byte 0x81 in position 38: invalid start byte

Changes proposed in this PR:

  • Add errors='ignore' to the ASL solver version check subprocess call. This will ignore any errors while trying to decode the data received from the -v command (ie. errors arising from special characters, unexpected bytes, etc). After all, Pyomo does not care about the value of the solver version; the version check appears to only verify the solver exists before attempting to solve - See opt/base/solvers.py and solvers/plugins/solvers/ASL.py for more details. You can also read about the errors argument to subprocess here: https://docs.python.org/3.10/library/io.html#io.TextIOWrapper

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.

Copy link
Copy Markdown
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.

This looks good. It would be nice if we had a way of testing this (i.e., testing against solvers compiled in any LOCALE other than en_US - as this is not the first time we have hit issues running in languages other than English), but I don't think that should prevent merging.

We should also consider changing the universal_newlines=True to test=True in that subprocess.run() call (text is the documented argument, and universal_newlines is now being provided for backwards compatibility).

@blnicho blnicho self-requested a review July 2, 2024 19:07
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.46%. Comparing base (17f8aed) to head (c8bf131).
Report is 889 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3298   +/-   ##
=======================================
  Coverage   88.46%   88.46%           
=======================================
  Files         867      867           
  Lines       98218    98218           
=======================================
+ Hits        86886    86887    +1     
+ Misses      11332    11331    -1     
Flag Coverage Δ
linux 86.27% <ø> (ø)
osx 75.56% <ø> (ø)
other 86.46% <ø> (ø)
win 83.77% <ø> (ø)

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.

@mrmundt mrmundt merged commit f5e6f32 into Pyomo:main Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants