Optimize ParamResolver.value_of

[maffoo]

This cleans up the logic in ParamResolver.value_of and makes a few optimizations:

• Throughout ParamResolver, refer to self._param_dict instead of the self.param_dict property.
• In ParamResolver.value_of, handle the cases of resolving a string or symbol in a more straightforward way by checking for their presence in the param dict (when resolving a string we check for both a str and symbol, and similarly when resolving a symbol). If the str or symbol is not found, we can immediately return the symbol without calling through to sympy substitution logic, which was being invoked previously. If the value is found, we check whether it is a "pass through" resolvable value and whether it needs to be resolved recursively and then return.

These changes ensure that sympy substitution logic is not invoked unless trying to resolve a complex expression, which fixes test_value_of_substituted_types so that it doesn't actually invoke substitution on basic symbols (the test was claiming to assert this while actually asserting the opposite), and also greatly improves performance when resolving missing symbols. On master, getting the value of a missing symbol invokes sympy and is quite slow:

In [1]: import cirq, sympy
In [2]: %time cirq.ParamResolver({"a": 1, "b": 2}).value_of(sympy.Symbol("foo"))
CPU times: user 843 µs, sys: 691 µs, total: 1.53 ms
Wall time: 1.55 ms

while on this branch the same operation is more than 10x faster:

In [1]: import cirq, sympy
In [2]: %time cirq.ParamResolver({"a": 1, "b": 2}).value_of(sympy.Symbol("foo"))
CPU times: user 69 µs, sys: 56 µs, total: 125 µs
Wall time: 130 µs

Note that I also slightly changed the behavior of resolving a type with an explicit _resolved_value_ method that returns NotImplemented. Previously this would raise an obscure sympy error, but here I've changed things so that the behavior of such an "explicitly not implemented" method is the same as if the method were omitted entirely (actually not implemented). In the latter case we were previously returning the original symbol, and now we do that for both cases. I think it's debatable whether we should in fact return the symbol in such cases, but this at least makes things more consistent.

[maffoo]

Not sure why we decided to always return a symbol here even if resolving a string. My preference would be to return the given value unchanged if the resolver doesn't need to change it, but callers could rely on this behavior so we should be careful if we want to change it.

[maffoo]

This was coincidentally working before because the value_of code was calling float on the result after looking up in the param dict. But I think that behavior was unintentional because otherwise there would be no way to distinguish between strings used as symbols and strings containing floats. I can't think of any other place where we allow floats to be specified as strings, outside of serialization, and it certainly wasn't the intent to do so for parameter resolution.

[andbe91]

Any chance of getting this into master this week? Would be great for accelerating some internal efforts :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0e288a7) 97.84% compared to head (68631ed) 97.84%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6341      +/-   ##
==========================================
- Coverage   97.84%   97.84%   -0.01%     
==========================================
  Files        1110     1110              
  Lines       96597    96656      +59     
==========================================
+ Hits        94516    94572      +56     
- Misses       2081     2084       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dstrain115]

Shouldn't global variables be named like _NOT_FOUND according to the style guide? Same with RecursionFlag below if you want to clean it up.
go/pystyle#guidelines-derived-from-guidos-recommendations

[maffoo]

Renamed to _NOT_FOUND and _RECURSION_FLAG.