Resolve NLv2 incompatibility with multithreading #3332
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3332 +/- ##
==========================================
+ Coverage 88.52% 88.70% +0.17%
==========================================
Files 868 878 +10
Lines 98436 101165 +2729
==========================================
+ Hits 87144 89739 +2595
- Misses 11292 11426 +134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
mrmundt
reviewed
Aug 5, 2024
pyomo/repn/ampl.py
Outdated
| } | ||
|
|
||
|
|
||
| class text_nl_debug_template(object): |
There was a problem hiding this comment.
Also this class name violates our own standards. I get that this is a legacy thing, but we may want to find places where we violate our own rules at some point.
| _create_strict_inequality_map(vars()) | ||
|
|
||
|
|
||
| class NLFragment(object): |
| return 'nl(' + self._node.name + ')' | ||
|
|
||
|
|
||
| class AMPLRepn(object): |
mrmundt
approved these changes
Aug 12, 2024
mrmundt
approved these changes
Aug 12, 2024
emma58
approved these changes
Aug 16, 2024
blnicho
reviewed
Aug 19, 2024
Co-authored-by: Bethany Nicholson <[email protected]>
blnicho
approved these changes
Aug 19, 2024
Co-authored-by: Bethany Nicholson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3329.
Summary/Motivation:
The NLv2 writer makes use of a visitor to both walk / compile the model expressions and to generate the string representation of the nonlinear portion of expressions. To accommodate this, the
AMPLRepnresult needed a handle on the visitor to be able to know what the current template is (terse or annotated). We implemented this using a module-level singleton, which works for general serial Python processes, but is not thread-safe. This PR redesigns the AMPLRepn so that the template belongs to the repn class and not the visitor, thereby removing the need for the singleton.As a bonus, removing the singleton allows us to more easily break the visitor out of the writer module and into a separate
pyono.repn.amplmodule (following the pattern of the LP writer)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: