Fix regex pattern for performance log parsing

[Saiamd999]

Motivation

we have (',' and ‘/s’) after the perf numbers which breaks the regex pattern matching in the madengine framework.
fixes: https://amd-hub.atlassian.net/browse/ROCM-21596

Technical Details

Test Plan

Test Result

Uploading pyt_davit_singlegpu_pyt_davit.ubuntu.amd.run.live.log…
jax_resnet_jax_flax.ubuntu.amd.run.live.log

Submission Checklist

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

[Copilot]

Pull request overview

Updates MADEngine’s performance log parsing to tolerate additional suffix/punctuation after the numeric performance value (e.g., , and /s) so performance extraction works with the updated log format mentioned in ROCM-21596.

Changes:

• Relaxed the performance: regex to allow optional /s-like suffixes and an optional comma after the numeric value.
• Strips a trailing comma from the parsed metric token.

---

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

[Copilot]

The new performance regex is overly permissive: [/a-zA-Z]* will accept arbitrary alphabetic suffixes on the numeric value (e.g., performance: 12abc samples_per_second will now parse as 12.0), which can silently mis-parse malformed logs. Since the stated goal is to handle a trailing "," and/or "/s", consider restricting this portion to those exact tokens (and, if needed, support both orders like "/s," vs ",/s") instead of allowing any letters.

Suggested change

	perf_pattern = r"performance:\s*([\d.]+)[/a-zA-Z]*,?\s+(\S+)"
	perf_pattern = r"performance:\s*([\d.]+)(?:/s,|,/s|/s|,)?\s+(\S+)"

[Copilot]

This parsing change is behaviorally significant (new supported formats like trailing "/s" and commas). Please add unit tests for _parse_performance_from_log covering the new formats (e.g., performance: 12.34/s samples_per_second, performance: 12.34, samples_per_second, and metric with a trailing comma) to prevent regressions; there are already unit tests for other helpers in tests/unit/test_deployment.py.

[srinivamd]

Code Review — PR#102: Fix regex pattern for performance log parsing

Context: This PR was analyzed as part of ROCM-21596 investigation.

Summary

The fix correctly addresses the root cause: the performance log parser in _parse_performance_from_log() used a regex that required whitespace immediately after the numeric value, but several model wrappers emit performance: VALUE, LABEL (comma between value and label). The relaxed regex and .rstrip(',') cleanup resolve all four reported models.

Review Notes

Change 1 — Regex relaxation (✅ Correct, minor improvement possible)

-        perf_pattern = r"performance:\s*([\d.]+)\s+(\S+)"
+        perf_pattern = r"performance:\s*([\d.]+)[/a-zA-Z]*,?\s+(\S+)"

The new pattern handles the reported performance: 0.0068, loss case correctly:

• ([\d.]+) captures 0.0068
• [/a-zA-Z]* matches nothing (no unit attached to the number)
• ,? matches the ,
• \s+ matches the space
• (\S+) captures loss

Minor suggestion: [/a-zA-Z]* excludes digits, so a unit like tok/s2 or any alphanumeric suffix would not be fully consumed. A more general alternative:

perf_pattern = r"performance:\s*([\d.]+)[^,\s]*,?\s+(\S+)"

[^,\s]* (zero or more characters that are not a comma or whitespace) handles any unit suffix without enumerating character classes. This is a non-blocking suggestion — the current fix resolves all reported cases.

Change 2 — Metric cleanup (✅ Correct)

-        metric = match.group(2)
+        metric = match.group(2).rstrip(',')

Correctly strips trailing commas from the metric token. No issues here.

No regression risk: existing valid performance: NUMBER METRIC log lines continue to match under the new pattern.

Verdict

Approve with optional suggestion. The fix is correct and safe to merge. The [^,\s]* alternative is a minor robustness improvement that could be applied now or in a follow-up.

Review generated with AI assistance (ROCM-21596 investigation).