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

Ensure alpha conversion always runs when a new variable is bound #414

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
02b5d9d
Ensure alpha conversion always runs when a new variable is bound
eb8680 Dec 25, 2020
6135a8b
add regression test
eb8680 Dec 28, 2020
cdf8ad0
further code organization change
eb8680 Dec 28, 2020
439d63e
dont strip name
eb8680 Dec 28, 2020
3fba6dd
simplify, remove hash() usage
eb8680 Dec 28, 2020
aa4c89c
simplify alpha_mangle
eb8680 Dec 28, 2020
b5da767
optimization to avoid some unnecessary deep recursion in Funsor._alph…
eb8680 Dec 28, 2020
73d2d50
optimization to avoid unnecessary recursion in Contraction._alpha_con…
eb8680 Dec 28, 2020
5cc64d5
nit
eb8680 Dec 28, 2020
7ba9869
Merge branch 'master' into repeat-alpha-conversion
eb8680 Feb 17, 2021
610037b
fix test
eb8680 Feb 17, 2021
e642186
Merge branch 'master' into repeat-alpha-conversion
eb8680 Feb 21, 2021
c42834d
try forcing reflect in _alpha_convert
eb8680 Feb 21, 2021
b4bed63
centralize reflect application in alpha conversion
eb8680 Feb 21, 2021
9c80304
centralize alpha_convert further
eb8680 Mar 16, 2021
ccfdec5
remove obsolete alpha_convert methods
eb8680 Mar 16, 2021
e982f17
Merge branch 'master' into repeat-alpha-conversion
eb8680 Mar 18, 2021
9b6a60a
delete more _alpha_convert methods
eb8680 Mar 18, 2021
fa79950
lint
eb8680 Mar 18, 2021
d2ca611
lint
eb8680 Mar 18, 2021
6456184
remove outdated comment
eb8680 Mar 18, 2021
c4e8010
Merge branch 'master' into repeat-alpha-conversion
eb8680 Apr 9, 2021
cd4fade
Merge branch 'master' into repeat-alpha-conversion
eb8680 Apr 13, 2021
01bfd39
avoid renaming twice
eb8680 Apr 13, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions funsor/terms.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,12 @@ def subs_interpreter(cls, *args):
return interpreter.reinterpret(expr)


def _alpha_mangle(expr):
def _alpha_mangle(bound_vars):
"""
Rename bound variables in expr to avoid conflict with any free variables.

FIXME this does not avoid conflict with other bound variables.
Returns substitution dictionary with mangled names for consumption by Funsor._alpha_convert.
"""
alpha_subs = {name: interpreter.gensym(name + "__BOUND")
for name in expr.bound if "__BOUND" not in name}
if not alpha_subs:
return expr

ast_values = expr._alpha_convert(alpha_subs)
return reflect(type(expr), *ast_values)
return {name: interpreter.gensym(name.split("__BOUND_")[0] + "__BOUND_") for name in bound_vars}


def reflect(cls, *args, **kwargs):
Expand Down Expand Up @@ -81,8 +74,24 @@ def reflect(cls, *args, **kwargs):
result = super(FunsorMeta, cls_specific).__call__(*args)
result._ast_values = args

# alpha-convert eagerly upon binding any variable
result = _alpha_mangle(result)
# alpha-convert eagerly upon binding any variable.
# the identifier we use to reconcile alpha-conversion and cons-hashing
# is the string literal of hash() of the type and cons-hashing key:
if result.bound:
alpha_subs = _alpha_mangle(result.bound)
alpha_mangled_args = result._alpha_convert(alpha_subs)

# TODO eliminate code duplication below
# this is currently necessary because .bound is computed in __init__().
result = super(FunsorMeta, cls_specific).__call__(*alpha_mangled_args)
result._ast_values = alpha_mangled_args

# we also make the old cons cache_key point to the new mangled value.
# this guarantees that alpha-conversion only runs once for this expression.
cls._cons_cache[cache_key] = result
Copy link
Member Author

Choose a reason for hiding this comment

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

This line ensures that _alpha_mangle does not break cons-hashing.


cache_key = tuple(id(arg) if type(arg).__name__ == "DeviceArray" or not isinstance(arg, Hashable)
else arg for arg in alpha_mangled_args)

cls._cons_cache[cache_key] = result
return result
Expand Down
8 changes: 8 additions & 0 deletions test/test_alpha_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,11 @@ def test_sample_independent():
actual = Independent(f, 'x', 'i', 'x_i')
assert actual.sample('i')
assert actual.sample('j', {'i': 2})


def test_subs_already_bound():
with interpretation(reflect):
x = Variable('x', Real)
y1 = (2 * x)(x=3)
y2 = y1.arg(4)
assert y1.bound != y2.bound