Exploit repeated named expressions in identify_variables

[Robbybp]

Fixes #3187

Summary/Motivation:

identify_variables can be responsible for a non-trivial portion of runtime for models with many occurrences of large named expressions. This PR adds a new variable visitor that supports descend_into_named_expressions=False and updates get_vars_from_components to use this new visitor to avoid re-checking the same named expression multiple times.

The potential performance hit is most pronounced when identifying variables for many expressions in a model, so I've only used this visitor in get_vars_from_components. This functionality could also be useful in identify_variables, but I wanted to get some feedback before implementing this.

Edit: This PR now updates identify_variables.

Changes proposed in this PR:

• Add a new visitor for identifying variables in an expression (for now, called _StreamVariableVisitor... I'm open to suggestions)
• Update identify_variables to use this visitor and add an optional named_expression_cache argument
• Use named_expression_cache in get_vars_from_components

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.

[Robbybp]

Test failures are in a GDP baseline file comparison, likely due to changes in variable order generated by get_vars_from_components.

[blnicho]

We discussed a few implementation changes during the dev call today so I'm going to mark this as a draft for now

[Robbybp]

I had to update some tests to use ComponentSet to get test_visitor.py to work, so this doesn't give the same variable order that identify_variables used to. For that reason, I don't expect tests to pass yet.

I think the reason for the difference is that _VariableVisitor uses xbfs_yield_leaves, while StreamBasedExpressionVisitor seems to use DFS (at least in the recursive implementation).

[Robbybp]

With this patch, the motivating example from #3186 looks pretty good:

Identifier                                     ncalls   cumtime   percall      %
--------------------------------------------------------------------------------
root                                                1     2.348     2.348  100.0
     ---------------------------------------------------------------------------
     full model post-solve                          1     0.460     0.460   19.6
     solve-scc                                      1     1.888     1.888   80.4
                          ------------------------------------------------------
                          identify-variables      546     0.188     0.000   10.0
                          other                   n/a     1.700       n/a   90.0
                          ======================================================
     other                                        n/a     0.000       n/a    0.0
     ===========================================================================
================================================================================

[codecov]

Codecov Report

Attention: Patch coverage is 96.29630% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 88.40%. Comparing base (c849905) to head (22e11e0).
Report is 146 commits behind head on main.

Files	Patch %	Lines
pyomo/core/expr/visitor.py	96.05%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3190   +/-   ##
=======================================
  Coverage   88.40%   88.40%           
=======================================
  Files         846      846           
  Lines       95156    95207   +51     
=======================================
+ Hits        84118    84169   +51     
  Misses      11038    11038           

Flag	Coverage Δ
linux	86.34% <93.82%> (+<0.01%)	⬆️
osx	76.18% <93.82%> (+0.01%)	⬆️
other	86.54% <93.82%> (+<0.01%)	⬆️
win	83.84% <93.82%> (+<0.01%)	⬆️

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.

[jsiirola]

I think there is a significantly more compact implementation of this, but there is certainly nothing wrong with this. I propose merging this, and defer improvements into a later PR.