Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/node probe #463

Merged
merged 4 commits into from Apr 20, 2021
Merged

Feature/node probe #463

merged 4 commits into from Apr 20, 2021

Conversation

mpu-creare
Copy link
Contributor

Adds a utility method to probe a node/pipeline at a point. It evaluates EVERY node in the pipeline, even if it is not being used.

See the unit tests for expected usage.

@mpu-creare mpu-creare requested review from jmilloy and drc-creare and removed request for drc-creare April 19, 2021 16:45
podpac/core/utils.py Outdated Show resolved Hide resolved
@mpu-creare
Copy link
Contributor Author

@jmilloy could you please spare a few minutes to review and merge when happy.

@coveralls
Copy link

Coverage Status

Coverage decreased (-88.4%) to 0.0% when pulling 35adfd0 on feature/node_probe into b43613f on develop.

@coveralls
Copy link

coveralls commented Apr 19, 2021

Coverage Status

Coverage decreased (-0.7%) to 87.751% when pulling cb2d494 on feature/node_probe into b43613f on develop.

…emoving depency on cache for Compositor case.
@mpu-creare mpu-creare self-assigned this Apr 19, 2021
@mpu-creare
Copy link
Contributor Author

@jmilloy let me know if/when you are happy and I'll merge.

@mpu-creare mpu-creare added this to In progress in Priorities via automation Apr 19, 2021
@mpu-creare mpu-creare added the enhancement New feature or request label Apr 19, 2021
@mpu-creare mpu-creare linked an issue Apr 19, 2021 that may be closed by this pull request
Copy link
Collaborator

@jmilloy jmilloy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I just had one question about the OrderedCompositer active loop.

podpac/core/utils.py Show resolved Hide resolved
@mpu-creare mpu-creare merged commit 9f911c2 into develop Apr 20, 2021
Priorities automation moved this from In progress to Done Apr 20, 2021
@mpu-creare mpu-creare deleted the feature/node_probe branch April 20, 2021 14:21
@jmilloy
Copy link
Collaborator

jmilloy commented Apr 20, 2021

I added a test for the case I'm concerned about. Now I think I get what you are doing (you check the source values and mark the source active if it matches the orderedcompositor value), so I was able to make a test that fails. See d812b92.

@jmilloy
Copy link
Collaborator

jmilloy commented Apr 20, 2021

Hold on, I jumped the gun. Just a sec.

@mpu-creare
Copy link
Contributor Author

Oh True... I see what you mean... Yeah, if two sources give the same value they would both be marked active. That does need a fix.

@jmilloy
Copy link
Collaborator

jmilloy commented Apr 20, 2021

Okay, I fixed the test. It now fails only because of the active flag.

@mpu-creare
Copy link
Contributor Author

ok, let me fix it.

@jmilloy
Copy link
Collaborator

jmilloy commented Apr 20, 2021

It was a bit tricky because if two OrderedCompositor sources are the same (not object, but definition/hash), only one of them shows up in the compositor definition. So of course only one shows up in the probe output. I'm not sure if that should be considered a bug or a feature!

>>> a = podpac.data.Array(source=np.ones(3), coordinates=podpac.Coordinates([[0, 1, 2]], dims=['time'])) 
>>> b = podpac.data.Array(source=np.ones(3), coordinates=podpac.Coordinates([[0, 1, 2]], dims=['time'])) 
>>> c = podpac.compositor.OrderedCompositor(sources=[a, b])
>>> c.probe(time=1)
OrderedDict([('Array', {'active': True, 'value': 1.0, 'inputs': [], 'name': 'Array', 'node_hash': 'e032d95041f8076a2a6317c079ace615'}), ('OrderedCompositor', {'active': True, 'value': 1.0, 'inputs': ['Array', 'Array'], 'name': 'OrderedCompositor', 'node_hash': '98354d0bc306ec5c8657590e04c0969a'})])

@jmilloy
Copy link
Collaborator

jmilloy commented Apr 20, 2021

Here's another fun one. Fails because the class isn't available in the global scope:

>>> def fn():
...     class AnotherOne(podpac.algorithm.Algorithm):
...         def algorithm(self, inputs, coordinates):
...             return self.create_output_array(coordinates, data=1)
...     c = podpac.compositor.OrderedCompositor(sources=[a, b, AnotherOne()])
...     c.probe(time=1)
... 
>>> fn()
Traceback (most recent call last):
  File "/home/jmilloy/Creare/Pipeline/podpac/podpac/core/node.py", line 816, in from_definition
    node_class = getattr(module, node_name)
AttributeError: module '__main__' has no attribute 'AnotherOne'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 6, in fn
  File "/home/jmilloy/Creare/Pipeline/podpac/podpac/core/node.py", line 418, in probe
    return probe_node(self, lat, lon, time, alt)
  File "/home/jmilloy/Creare/Pipeline/podpac/podpac/core/utils.py", line 556, in probe_node
    n = podpac.Node.from_definition(d)
  File "/home/jmilloy/Creare/Pipeline/podpac/podpac/core/node.py", line 818, in from_definition
    raise ValueError(
ValueError: Invalid definition for node 'AnotherOne': class 'AnotherOne' not found in module '__main__'

In the test, I had to define the AnotherOne class globally instead of in the test method.

Again, not sure if this is a bug or just the way things are...

@mpu-creare
Copy link
Contributor Author

I'll call it a feature. :) Fixed in d938580 Thanks for flagging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Priorities
  
Done
Development

Successfully merging this pull request may close these issues.

Point Prober functionality
3 participants