Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

### 🚀 Features

- Add `filter_name` argument to the attribute setting convenience function of Segment, so it's possible to filter not only by type but also by name. (see #598) (@jank324)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Needs to be moved to new version

- New `get_attr_from_every_element` method on `Segment` to retrieve attributes from all elements, with optional filtering by type and/or name. (see #598) (@jank324)
Comment thread
jank324 marked this conversation as resolved.

### 🐛 Bug fixes

### 🐆 Other
Expand Down
54 changes: 48 additions & 6 deletions cheetah/accelerator/segment.py
Original file line number Diff line number Diff line change
Expand Up @@ -584,28 +584,70 @@ def get_beam_attrs_along_segment(
else broadcasted_results[0]
)

def get_attr_from_every_element(
self,
attr_name: str,
filter_type: type[Element] | tuple[type[Element]] | None = None,
filter_name: str | None = None,
Comment thread
jank324 marked this conversation as resolved.

Copilot AI Dec 4, 2025

Copy link

Choose a reason for hiding this comment

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

Inconsistent type annotation for filter_name parameter. In get_attr_from_every_element, it's typed as str | None, but in set_attrs_on_every_element (line 622), it's typed as str | tuple[str] | None. These should be consistent. The setter allows a tuple to support multiple names, and the getter should do the same for consistency.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

is_recursive: bool = True,
) -> list[Any]:
"""
Get an attribute from every element in the segment filtered by type and/or name.
:param attr_name: Name of the attribute to retrieve from each element.
:param filter_type: Type of the elements to get the attribute from.
:param filter_name: Name of the elements to get the attribute from.
:param is_recursive: If `True`, this method is applied to nested `Segment`s as
well. If `False`, only the elements directly in the top-level `Segment` are
considered.
:return: List of attribute values from the filtered elements.
"""
attrs = []
for element in self.elements:
if (filter_type is None or isinstance(element, filter_type)) and (
filter_name is None or element.name in filter_name
):
attrs.append(getattr(element, attr_name))
elif is_recursive and isinstance(element, Segment):
attrs.extend(
element.get_attr_from_every_element(
Comment thread
jank324 marked this conversation as resolved.
attr_name,
filter_type=filter_type,
filter_name=filter_name,
is_recursive=True,
)
)
return attrs

def set_attrs_on_every_element(
self,
filter_type: type[Element] | tuple[type[Element]] | None = None,
filter_name: str | tuple[str] | None = None,
is_recursive: bool = True,
**kwargs: dict[str, Any],
) -> None:
"""
Set attributes on every element of a specific type in the segment.
Set attributes on every element type in the segment filtered by type and/or
name.

:param filter_type: Type of the elements to set the attributes for.
:param is_recursive: If `True`, the this method is applied to nested `Segment`s
as well. If `False`, only the elements directly in the top-level `Segment`
are considered.
:param filter_name: Name of the element to set the attributes for.
:param is_recursive: If `True`, this method is applied to nested `Segment`s as
well. If `False`, only the elements directly in the top-level `Segment` are
considered.
:param kwargs: Attributes to set and their values.
"""
for element in self.elements:
if filter_type is None or isinstance(element, filter_type):
if (filter_type is None or isinstance(element, filter_type)) and (
filter_name is None or element.name in filter_name
):
for key, value in kwargs.items():
setattr(element, key, value)
elif is_recursive and isinstance(element, Segment):
element.set_attrs_on_every_element(
filter_type=filter_type, is_recursive=True, **kwargs
filter_type=filter_type,
filter_name=filter_name,
is_recursive=True,
**kwargs,
)
Comment on lines +640 to 651

Copilot AI Dec 4, 2025

Copy link

Choose a reason for hiding this comment

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

Missing test coverage for the new filter_name parameter. While tests exist for filtering by type, the new name filtering functionality is not tested.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback


def plot(
Expand Down