Skip to content

Tweak get_var_by_name #1440

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

Open
ricardoV94 opened this issue Jun 3, 2025 · 1 comment · May be fixed by #1441
Open

Tweak get_var_by_name #1440

ricardoV94 opened this issue Jun 3, 2025 · 1 comment · May be fixed by #1441

Comments

@ricardoV94
Copy link
Member

Description

This function has an unused argument: ids.

More importantly it also looks for matches in inner_graphs. I would make this behavior optional and non-default, since variables in inner_graphs are not really variables of the graph.

def get_var_by_name(
graphs: Iterable[Variable], target_var_id: str, ids: str = "CHAR"
) -> tuple[Variable, ...]:
r"""Get variables in a graph using their names.
Parameters
----------
graphs:
The graph, or graphs, to search.
target_var_id:
The name to match against either ``Variable.name`` or
``Variable.auto_name``.
Returns
-------
A ``tuple`` containing all the `Variable`\s that match `target_var_id`.
"""
from pytensor.graph.op import HasInnerGraph
def expand(r) -> list[Variable] | None:
if not r.owner:
return None
res = list(r.owner.inputs)
if isinstance(r.owner.op, HasInnerGraph):
res.extend(r.owner.op.inner_outputs)
return res
results: tuple[Variable, ...] = ()
for var in walk(graphs, expand, False):
var = cast(Variable, var)
if target_var_id == var.name or target_var_id == var.auto_name:
results += (var,)
return results

@prajwalun prajwalun linked a pull request Jun 5, 2025 that will close this issue
11 tasks
@prajwalun
Copy link

@ricardoV94 Opened a PR to address this issue: #1441. Let me know if any further changes or test refinements are needed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants