SWDEV-551979 Fixing kernel filtering gui analyze mode. by ggottipa-amd · Pull Request #2210 · ROCm/rocm-systems

ggottipa-amd · 2025-12-08T12:37:07Z

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

vedithal-amd · 2025-12-09T16:44:46Z

-            base_data[base_run].filter_kernel_ids = kernel_ids
-
+            base_data[base_run].filter_kernel_ids = (
+                [str(k) for k in kernel_filter] if kernel_filter else []


you dont need if here

if kernel_filter is empty list it will be automatically handled

just make sure kernel_filter cannot be None

vedithal-amd · 2025-12-09T16:46:37Z

@@ -213,13 +185,24 @@ def generate_from_filter(
                    for key in base_data[base_run].dfs
                    if key in basic_dfs_keep
                }
-                base_data[base_run].dfs = filtered_dfs
+                # base_data[base_run].dfs = filtered_dfs


remove comments of unused code in production code

vedithal-amd

We should add in motivation section what change we are making and why.
Also let's add testing methodology and make sure we test properly.
We should also add screenshot of UI before and after making this change to better understand the PR

from webui.py .

Copilot

Pull request overview

This PR fixes kernel filtering functionality in GUI analyze mode by addressing issues with denominator handling, NaN value processing, and kernel filter type management.

Key Changes:

Fixed GRBM_GUI_ACTIVE_PER_XCD denominator reference by removing incorrect $ prefix
Enhanced NaN and "N/A" value handling throughout metric evaluation and GUI display
Modified kernel filtering to properly handle string kernel names from GUI dropdowns

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
projects/rocprofiler-compute/src/utils/parser.py	Fixed denominator key, improved NaN handling in metric evaluation, added case-insensitive field comparison, and added error handling for unsupported normalization units
projects/rocprofiler-compute/src/utils/gui.py	Extended filtering conditions to handle "N/A" and None values in chart generation
projects/rocprofiler-compute/src/roofline.py	Added fallback figure creation when no kernel data is available
projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_webui.py	Modified kernel filter handling to preserve string types from GUI and updated filtered dataframes logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T08:28:20Z

+        """Evaluate a single expression with proper local context."""                
+


Trailing whitespace detected at the end of this line. This should be removed to maintain clean code formatting.

Suggested change

"""Evaluate a single expression with proper local context."""

"""Evaluate a single expression with proper local context."""

Copilot · 2025-12-19T08:28:20Z

+            if 'denom' in expr:
+                console_warning(f"Evaluated expression '{expr}' with result: {eval_result}")
+


Debug console_warning statements should be removed before merging. These appear to be temporary debugging code left in the codebase.

Suggested change

if 'denom' in expr:

console_warning(f"Evaluated expression '{expr}' with result: {eval_result}")

Copilot · 2025-12-19T08:28:21Z

+    else:
+        console_error(
+            f"Normalization unit '{normal_unit}' is not supported. "
+            "Skiped updating $denom."


Typo in error message: 'Skiped' should be 'Skipped'.

Suggested change

"Skiped updating $denom."

"Skipped updating $denom."

Copilot · 2025-12-19T08:28:21Z

+    if 'denom' in equation_string:
+        console_error(f"Updated equation with denom:{equation} --> {equation_string}")


Debug console_error statement should be removed before merging. This appears to be temporary debugging code that logs every equation update with denom.

Suggested change

if 'denom' in equation_string:

console_error(f"Updated equation with denom:{equation} --> {equation_string}")

Copilot · 2025-12-19T08:28:21Z

+            console_warning('kernel_filter',kernel_filter)
+            console_warning('base_data[base_run].filter_kernel_ids',base_data[base_run].filter_kernel_ids)
+


Debug console_warning statements should be removed before merging. These appear to be temporary debugging code left to track kernel filter values.

Suggested change

console_warning('kernel_filter',kernel_filter)

console_warning('base_data[base_run].filter_kernel_ids',base_data[base_run].filter_kernel_ids)

Copilot · 2025-12-19T08:28:22Z

                    if key in basic_dfs_keep
                }
-                base_data[base_run].dfs = filtered_dfs
+                # base_data[base_run].dfs = filtered_dfs


This line is commented out but filtered_dfs is still used later in the code. The assignment to filtered_dfs should either be uncommented or the variable should be renamed to avoid confusion about which dfs object is being used.

Suggested change

# base_data[base_run].dfs = filtered_dfs

base_data[base_run].dfs = filtered_dfs

Copilot · 2025-12-19T08:28:22Z

+            #     console_warning("Local context keys:",local_expr_context)
+            #     console_warning(f"=== EVAL DEBUG for: {expr} ===")
+
+            #     if 'raw_pmc_df' in local_expr_context:
+            #         pmc_df = local_expr_context['raw_pmc_df']
+            #         console_warning(f"raw_pmc_df type: {type(pmc_df)}")
+
+            #         grbm_col = pmc_df['pmc_perf']['GRBM_GUI_ACTIVE']
+            #         console_warning(f"GRBM_GUI_ACTIVE type: {type(grbm_col)}")
+            #         console_warning(f"GRBM_GUI_ACTIVE shape: {getattr(grbm_col, 'shape', 'No shape')}")
+            #         console_warning(f"GRBM_GUI_ACTIVE values: {grbm_col}")
+            #         console_warning("GRBM_GUI_ACTIVE index:",grbm_col.index)
+            #     else:
+            #         console_error("raw_pmc_df not found in local_expr_context.")
+
+            #     # Print num_xcd
+            #     num_xcd = local_expr_context.get('ammolite__num_xcd')
+            #     console_warning(f"ammolite__num_xcd type: {type(num_xcd)}")
+            #     console_warning(f"ammolite__num_xcd value: {num_xcd}")
+


This comment appears to contain commented-out code.

Suggested change

# console_warning("Local context keys:",local_expr_context)

# console_warning(f"=== EVAL DEBUG for: {expr} ===")

# if 'raw_pmc_df' in local_expr_context:

# pmc_df = local_expr_context['raw_pmc_df']

# console_warning(f"raw_pmc_df type: {type(pmc_df)}")

# grbm_col = pmc_df['pmc_perf']['GRBM_GUI_ACTIVE']

# console_warning(f"GRBM_GUI_ACTIVE type: {type(grbm_col)}")

# console_warning(f"GRBM_GUI_ACTIVE shape: {getattr(grbm_col, 'shape', 'No shape')}")

# console_warning(f"GRBM_GUI_ACTIVE values: {grbm_col}")

# console_warning("GRBM_GUI_ACTIVE index:",grbm_col.index)

# else:

# console_error("raw_pmc_df not found in local_expr_context.")

# # Print num_xcd

# num_xcd = local_expr_context.get('ammolite__num_xcd')

# console_warning(f"ammolite__num_xcd type: {type(num_xcd)}")

# console_warning(f"ammolite__num_xcd value: {num_xcd}")

vedithal-amd · 2026-01-02T21:48:05Z

This PR can be closed against #2080

ggottipa-amd · 2026-01-07T12:56:07Z

Closing this against #2479

ggottipa-amd requested a review from a team as a code owner December 8, 2025 12:37

github-actions Bot added the project: rocprofiler-compute label Dec 8, 2025

systems-assistant Bot added the organization: ROCm label Dec 8, 2025

vedithal-amd reviewed Dec 9, 2025

View reviewed changes

ggottipa-amd force-pushed the users/ggottipa-amd/fix-kernel-filtering-gui branch 2 times, most recently from 68d1fb4 to af57f86 Compare December 11, 2025 12:45

ggottipa-amd added 6 commits December 12, 2025 06:13

Converting kernel-names from web ui into kernel ids for filtering.

8816394

Removing debug prints.

3d1750e

Reverting the kernel id to kernel name conversion added in prev commit

2d2025f

from webui.py .

Using sum on GRBM counters to fix Error - unhashable type: 'Series'.

902f824

Removed dict while validating eval_result.

51d53ae

Fixing SUPPORTED_DENOM, removed $ from GRBM_GUI_ACTIVE_PER_XCD.

76c5be2

ggottipa-amd force-pushed the users/ggottipa-amd/fix-kernel-filtering-gui branch from cba5c4b to 76c5be2 Compare December 12, 2025 12:14

ggottipa-amd requested a review from Copilot December 19, 2025 08:23

Copilot started reviewing on behalf of ggottipa-amd December 19, 2025 08:24 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

ggottipa-amd closed this Jan 7, 2026

		"""Evaluate a single expression with proper local context."""

		if 'denom' in expr:
		console_warning(f"Evaluated expression '{expr}' with result: {eval_result}")

		if 'denom' in equation_string:
		console_error(f"Updated equation with denom:{equation} --> {equation_string}")

		console_warning('kernel_filter',kernel_filter)
		console_warning('base_data[base_run].filter_kernel_ids',base_data[base_run].filter_kernel_ids)

	# base_data[base_run].dfs = filtered_dfs
	base_data[base_run].dfs = filtered_dfs

Conversation

ggottipa-amd commented Dec 8, 2025

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

vedithal-amd Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

vedithal-amd Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

vedithal-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

vedithal-amd commented Jan 2, 2026

Uh oh!

ggottipa-amd commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants