mosek_direct.py : Fixed iparam parsing and added support for Task.putparam#3488
mosek_direct.py : Fixed iparam parsing and added support for Task.putparam#3488mrmundt merged 17 commits intoPyomo:mainfrom
Conversation
…ameter and the parameter value as strings, as long as the "generic" parameter names are used. Fixed: call to putintparam was missing in the isinstance(option, str) conditional when parsing 'iparam'.
|
@Utkarsh-Detha - please run |
|
Thank you for the review, @mrmundt . I ran the formatter as you recommended, with the newest version thereof. |
|
@Utkarsh-Detha could you add a couple tests for these changes? |
|
@Utkarsh-Detha - Pinging again about this! Could you please add in some tests? |
jsiirola
left a comment
jsiirola
left a comment
There was a problem hiding this comment.
Overall this looks reasonable, but could be improved by some defensive checks. In addition, it would be good to test this functionality thought the unit tests.
|
@jsiirola I am starting to wonder if I should simply remove the option of allowing strings as iparam values. The idea initially was to allow users to pass MOSEK enums as strings, for example |
2. Added a test to check if an AttributeError is raised when wrong parameter names are passed in options.
jsiirola
left a comment
jsiirola
left a comment
There was a problem hiding this comment.
I think this is really close. It would be good to add one extra check and clarify the error message. If you have a chance to address this in the next couple hours, great - otherwise I may try and push to this branch and try to squeeze it into tomorrow's release.
|
@Utkarsh-Detha Good question. I don't have a strong opinion on supporting strings for |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3488 +/- ##
==========================================
+ Coverage 88.73% 88.75% +0.02%
==========================================
Files 890 890
Lines 102207 102209 +2
==========================================
+ Hits 90692 90715 +23
+ Misses 11515 11494 -21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
… instead of warnings for wrong input type 2. Removed call to "split" before searching for string pattern in key names
|
@jsiirola I also added/changed some tests to check if the correct exceptions are raised in case wrong types are passed for values of params. If a "str" type is passed to iparam but the name is recognized as a valid MOSEK enum, then an AttributeError is raised, which is also tested for in the tests. Let me know if you don't think this is a reasonable approach. Thanks for your help on this. |
github-project-automation
bot
moved this from Review In Progress
to Reviewer Approved
in Pyomo Release April 2025
github-project-automation
bot
moved this from Reviewer Approved
to Done
in Pyomo Release April 2025
5 participants
Fixes:
In
_apply_solverputintparamwas missing in theisinstance(option, str)conditional when parsingiparam. Fixed.option.pop('mosek')instead ofoption.pop(0)within the same conditional. Fixed.Summary/Motivation:
A user wanted to be able to pass the value of ALL parameters as strings. MOSEK's optimizer API supports this through the
Task.putparamfunction, which accepts a parameter name (string) and a value (also string).Changes proposed in this PR:
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: