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

### 🚀 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


### 🐛 Bug fixes

### 🐆 Other
Expand Down
11 changes: 9 additions & 2 deletions cheetah/accelerator/segment.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,25 +587,32 @@ def get_beam_attrs_along_segment(
def set_attrs_on_every_element(
self,
filter_type: type[Element] | tuple[type[Element]] | None = None,
filter_name: str | None = None,
Comment thread
jank324 marked this conversation as resolved.
is_recursive: bool = True,
**kwargs: dict[str, Any],
) -> None:
"""
Set attributes on every element of a specific type in the segment.

:param filter_type: Type of the elements to set the attributes for.
:param filter_name: Names of the elements to set the attributes for.
Comment thread
jank324 marked this conversation as resolved.
Outdated
:param is_recursive: If `True`, the this method is applied to nested `Segment`s
Comment thread
jank324 marked this conversation as resolved.
Outdated
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 == 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 +591 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.

The new filter_name parameter lacks test coverage. Since the existing function set_attrs_on_every_element has test coverage in tests/test_segment.py (see test_attr_setting_by_element_type_convenience_method), the new functionality should also be tested.

Consider adding a test that:

  1. Creates a segment with elements having specific names
  2. Calls set_attrs_on_every_element with the filter_name parameter
  3. Verifies that only elements matching the specified name have their attributes modified
  4. Tests the recursive behavior with nested segments

Copilot uses AI. Check for mistakes.
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