Add is_reference API #1740

Robbybp · 2020-12-09T01:49:23Z

Addresses #1640 .

Summary/Motivation:

Several users have noticed that there is no officially supported way to distinguish a component that was created by the Reference function. However, it is very useful to be able to make this distinction. Also, in some cases, it is useful to get the object ~~(slice)~~ from which a reference was created.

This PR addresses the first point by adding an ~~is_reference~~ owns_data method to PyomoObject, and the second by tacking on a referent attribute to the component returned by the Reference function.

Changes proposed in this PR:

Add owns_data method to PyomoObject and IndexedComponent
Add referent attribute to component returned by Reference
Use owns_data to address a hiccup in IndexedComponent that causes iteration over references-to-references to be incredibly slow.

One intent of this PR is to provide a concrete talking point for the proposals in #1640.

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:

I agree my contributions are submitted under the BSD license.
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.

codecov · 2020-12-10T16:38:10Z

Codecov Report

Merging #1740 (778aee5) into master (7f14b34) will increase coverage by 2.20%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1740      +/-   ##
==========================================
+ Coverage   72.76%   74.96%   +2.20%     
==========================================
  Files         629      638       +9     
  Lines       91423    91845     +422     
==========================================
+ Hits        66522    68855    +2333     
+ Misses      24901    22990    -1911

Impacted Files	Coverage Δ
pyomo/core/base/indexed_component.py	`87.11% <100.00%> (+0.14%)`	⬆️
pyomo/core/base/reference.py	`97.60% <100.00%> (+0.01%)`	⬆️
pyomo/core/pyomoobject.py	`94.44% <100.00%> (+0.69%)`	⬆️
pyomo/checker/plugins/checkers/model/rule.py	`73.68% <0.00%> (-17.43%)`	⬇️
pyomo/checker/runner.py	`88.60% <0.00%> (-9.97%)`	⬇️
pyomo/solvers/plugins/converter/model.py	`75.00% <0.00%> (-5.90%)`	⬇️
pyomo/repn/plugins/baron_writer.py	`73.74% <0.00%> (-3.64%)`	⬇️
examples/dae/simulator_ode_example.py	`50.00% <0.00%> (-3.34%)`	⬇️
examples/dae/simulator_ode_multindex_example.py	`60.65% <0.00%> (-3.14%)`	⬇️
pyomo/opt/parallel/pyro.py	`88.18% <0.00%> (-1.64%)`	⬇️
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f14b34...778aee5. Read the comment docs.

…e-api

Robbybp · 2020-12-28T20:06:59Z

As @jsiirola has pointed out, owns_data might be a better name than is_reference as the former is more general. In particular, if we ever want to decouple references and slices, we could end up with something like a SliceComponent function that creates something distinct from a "reference" that doesn't own its own data.

I've changed the method name here to reflect this and have added some tests.

michaelbynum · 2020-12-30T14:41:29Z

I like this a lot.

Robbybp · 2021-01-05T20:00:28Z

@carldlaird @michaelbynum @andrewlee94 We are trying to come up with the name for the method to test if an object owns its own data. Any suggestions on an is_* method that accomplishes this? Our best idea for now is is_reference, but this may not be general enough.

…e-api

blnicho

One minor question, otherwise this looks fine.

blnicho · 2021-01-15T21:57:06Z

pyomo/core/base/indexed_component.py

@@ -258,6 +258,10 @@ def is_indexed(self):
        """Return true if this component is indexed"""
        return self._index is not UnindexedComponent_set

+    def is_reference(self):
+        """ Return True if this component is a reference. """


Should we expand this description to include the idea of a reference "not owning" it's data objects?

I think that's a good idea. Have updated.

Robbybp added 3 commits December 8, 2020 20:02

referent attribute

aa770b3

base class is_reference method

064c51f

implement is_reference and fix for slow iteration over ref-to-ref

492fb91

jsiirola added the AT: PRE-TEST INSPECTED label Dec 10, 2020

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Dec 10, 2020

blnicho changed the title ~~Reference API~~ [WIP] Reference API Dec 16, 2020

Robbybp added 6 commits December 28, 2020 10:53

change method to owns_data

c26d16d

Merge branch 'master' of https://github.com/Pyomo/pyomo into referenc…

a3f23c9

…e-api

negate logic for owns_data

af7330c

assign referent before processing argument type

4a73994

test to cover base class owns_data

84a3943

tests for owns_data and referent attribute

42eba1a

Robbybp changed the title ~~[WIP] Reference API~~ Reference API Dec 31, 2020

blnicho requested review from jsiirola, blnicho and carldlaird January 5, 2021 19:48

Robbybp added 2 commits January 13, 2021 21:35

Merge branch 'master' of https://github.com/Pyomo/pyomo into referenc…

d886b72

…e-api

change name back to is_reference

98eb6fd

blnicho added the AT: PRE-TEST INSPECTED label Jan 15, 2021

blnicho approved these changes Jan 15, 2021

View reviewed changes

blnicho added AT: RETEST and removed AT: PRE-TEST INSPECTED labels Jan 15, 2021

expand docstring in is_reference

778aee5

blnicho added the AT: PRE-TEST INSPECTED label Jan 18, 2021

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Jan 18, 2021

blnicho removed the AT: RETEST label Jan 18, 2021

jsiirola approved these changes Jan 18, 2021

View reviewed changes

jsiirola added the AT: RETEST label Jan 19, 2021

pyomo-autotest removed the AT: RETEST label Jan 19, 2021

jsiirola changed the title ~~Reference API~~ Add is_reference API Jan 19, 2021

jsiirola merged commit dcd3fd8 into Pyomo:master Jan 19, 2021

Robbybp mentioned this pull request Feb 2, 2021

[PEP] Public API for References #1640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add is_reference API #1740

Add is_reference API #1740

Robbybp commented Dec 9, 2020 •

edited

Loading

codecov bot commented Dec 10, 2020 •

edited

Loading

Robbybp commented Dec 28, 2020

michaelbynum commented Dec 30, 2020

Robbybp commented Jan 5, 2021

blnicho left a comment

blnicho Jan 15, 2021

Robbybp Jan 16, 2021

Add is_reference API #1740

Add is_reference API #1740

Conversation

Robbybp commented Dec 9, 2020 • edited Loading

Addresses #1640 .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

codecov bot commented Dec 10, 2020 • edited Loading

Codecov Report

Robbybp commented Dec 28, 2020

michaelbynum commented Dec 30, 2020

Robbybp commented Jan 5, 2021

blnicho left a comment

Choose a reason for hiding this comment

blnicho Jan 15, 2021

Choose a reason for hiding this comment

Robbybp Jan 16, 2021

Choose a reason for hiding this comment

Robbybp commented Dec 9, 2020 •

edited

Loading

codecov bot commented Dec 10, 2020 •

edited

Loading