Skip to content

Allow string keys in eval utility #242

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 22, 2023
Merged

Allow string keys in eval utility #242

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b088b01
Added functionality that can use name of variable in eval
Raj-Parekh24 Mar 11, 2023
8976c94
Updated function with warning when multiple variables with same name …
Raj-Parekh24 Mar 11, 2023
b7f745e
Making convert_string_keys_to_variables internal to eval
Raj-Parekh24 Mar 14, 2023
8869a9d
Removed last variable selection and added test case for no name avail…
Raj-Parekh24 Mar 18, 2023
d889182
Merge branch 'main' into issue-227
Raj-Parekh24 Mar 20, 2023
6e2efaa
Modified the code structure based on suggestions
Raj-Parekh24 Mar 20, 2023
9008a32
Modified the code structure based on suggestions
Raj-Parekh24 Mar 20, 2023
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
22 changes: 22 additions & 0 deletions pytensor/graph/basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,28 @@ def eval(self, inputs_to_values=None):
if inputs_to_values is None:
inputs_to_values = {}

def convert_string_keys_to_variables():
process_input_to_values = {}
for i in inputs_to_values:
if isinstance(i, str):
nodes_with_matching_names = get_var_by_name([self], i)
length_of_nodes_with_matching_names = len(nodes_with_matching_names)
if length_of_nodes_with_matching_names == 0:
raise Exception(f"{i} not found in graph")
else:
if length_of_nodes_with_matching_names > 1:
raise Exception(
f"Found {length_of_nodes_with_matching_names} pytensor variables with name {i}"
)
process_input_to_values[
nodes_with_matching_names[0]
] = inputs_to_values[i]
else:
process_input_to_values[i] = inputs_to_values[i]
return process_input_to_values

inputs_to_values = convert_string_keys_to_variables()
Copy link
Member

Choose a reason for hiding this comment

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

It should be a bit more "pythonic" like this (I did not test the code!):

Suggested change
def convert_string_keys_to_variables():
process_input_to_values = {}
for i in inputs_to_values:
if isinstance(i, str):
nodes_with_matching_names = get_var_by_name([self], i)
length_of_nodes_with_matching_names = len(nodes_with_matching_names)
if length_of_nodes_with_matching_names == 0:
raise Exception(f"{i} not found in graph")
else:
if length_of_nodes_with_matching_names > 1:
raise Exception(
f"Found {length_of_nodes_with_matching_names} pytensor variables with name {i}"
)
process_input_to_values[
nodes_with_matching_names[0]
] = inputs_to_values[i]
else:
process_input_to_values[i] = inputs_to_values[i]
return process_input_to_values
inputs_to_values = convert_string_keys_to_variables()
def convert_string_keys_to_variables(input_to_values):
new_input_to_values = {}
for key, value in inputs_to_values.items():
if isinstance(key, str):
matching_vars = get_var_by_name([self], key)
if not matching_vars:
raise Exception(f"{key} not found in graph")
elif len(matching_vars) > 1:
raise Exception(
f"Found multiple variables with name {key}"
)
input_to_values[matching_vars[0]] = value
else:
new_input_to_values[key] = value
return new_input_to_values
inputs_to_values = convert_string_keys_to_variables(inputs_to_values)


if not hasattr(self, "_fn_cache"):
self._fn_cache = dict()

Expand Down
18 changes: 18 additions & 0 deletions tests/graph/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,24 @@ def test_eval(self):
pickle.loads(pickle.dumps(self.w)), "_fn_cache"
), "temporary functions must not be serialized"

def test_eval_with_strings(self):
assert self.w.eval({"x": 1.0, self.y: 2.0}) == 6.0
assert self.w.eval({self.z: 3}) == 6.0

def test_eval_errors_having_mulitple_variables_same_name(self):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def test_eval_errors_having_mulitple_variables_same_name(self):
def test_eval_with_strings_multiple_matches(self):

e = scalars("e")
t = e + 1
t.name = "e"
with pytest.raises(Exception, match="Found 2 pytensor variables with name e"):
t.eval({"e": 1})

def test_eval_errors_with_no_name_exists(self):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def test_eval_errors_with_no_name_exists(self):
def test_eval_with_strings_no_match(self):

e = scalars("e")
t = e + 1
t.name = "p"
with pytest.raises(Exception, match="o not found in graph"):
t.eval({"o": 1})


class TestAutoName:
def test_auto_name(self):
Expand Down