Resolve NLv2 incompatibility with multithreading#3332
Conversation
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. |
| } | ||
|
|
||
|
|
||
| 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
left a comment
There was a problem hiding this comment.
I still want you to remove the objects but I won't let it hold this up...
emma58
left a comment
There was a problem hiding this comment.
One docstring request, but this looks good.
Co-authored-by: Bethany Nicholson <[email protected]>
blnicho
left a comment
There was a problem hiding this comment.
I found one other typo but otherwise this looks fine
Co-authored-by: Bethany Nicholson <[email protected]>
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: