Omniprobe parser for bank conflicts #123

coleramos425 · 2025-07-12T20:04:42Z

Summary

This PR pulls in recent changes made to Omniprobe to support JSON output in the Memory Analysis pipeline, which improves the resiliency of Intelliperf to regex parsing errors
AMDResearch/omniprobe#26

Major changes:

pyproject.toml now points to the latest (corresponding) Omniprobe commit and ditches old SSH based cloning strategies in favor of HTTPS to decuple the crutch we had on ssh keys before. Updated Nexus to use the same technique when pulling kerneldb
New streamlined containerization setup which supports Apptainer and Docker from the same build script (containers/run.sh). Improved usability and enabled me to adequately test new Omniprobe functionality
Modified scripts/build_examples.sh and CMakeLists.txt to allow user to optionally specify path to their Omniprobe install for instrumentation support

Example:

intelliperf -vvv --project_directory=/intelliperf/examples --build_command="./scripts/build_examples.sh -c" --instrument_command="./scripts/build_examples.sh -i -c -o /intelliperf/external/omniprobe/install" --formula=bankConflict --provider https://llm-api.amd.com/azure --model dvue-aoai-001-o4-mini --num_attempts 1 -- ./build/bank_conflict/matrix_transpose 1024 1024

Instrument command uses new -o/--omniprobe option to specify path to Omniprobe install, which is used by CMake to build examples
I use AMD's LLM Gateway for my testing, but should work with your OpenAI API key too
--num-attempts set to 1 for brevity

Logs:
https://gist.github.com/coleramos425/567c7972647e02faa59d7977803f4652

Signed-off-by: coleramos425 <colramos@amd.com>

…nsolidated into a single run.sh script Signed-off-by: coleramos425 <colramos@amd.com>

Signed-off-by: coleramos425 <colramos@amd.com>

…have some notion of versioning Signed-off-by: coleramos425 <colramos@amd.com>

mawad-amd · 2025-07-13T04:20:06Z

CI is a bit fragile at the moment. Will fix soon.

containers/intelliperf.Dockerfile

containers/build.sh

Signed-off-by: coleramos425 <colramos@amd.com>

…ments Signed-off-by: coleramos425 <colramos@amd.com>

Signed-off-by: coleramos425 <colramos@amd.com>

…nt (first kernel) Signed-off-by: coleramos425 <colramos@amd.com>

…niprobe report with clone suffix Signed-off-by: coleramos425 <colramos@amd.com>

…ntended. Must modify shell cmd to cd to working_directory when specified Signed-off-by: coleramos425 <colramos@amd.com>

…cmds with new implementation Signed-off-by: coleramos425 <colramos@amd.com>

Signed-off-by: coleramos425 <colramos@amd.com>

external/guided-tuning/bin/gt

src/intelliperf/formulas/bank_conflict.py

mawad-amd · 2025-07-30T23:21:34Z

src/intelliperf/utils/process.py

 	verbose = logging.getLogger().getEffectiveLevel() <= logging.DEBUG

-	logging.debug(f"Running the command: {' '.join(subprocess_args)}")
+	# logging.debug(f"Running the command: {' '.join(subprocess_args)}")


Can we uncomment out this line?

Look at line 69 in the same file. It has been commented out here, because the to print the true value of subprocess_args the debug is best here

You can just remove the code commented out code then.

mawad-amd · 2025-07-30T23:24:13Z

src/intelliperf/utils/process.py

 	if additional_path is not None:
 		env["PATH"] = str(additional_path) + ":" + env["PATH"]

+	if working_directory is not None:


I am not sure what this logic is doing and why it is needed here. We can't simply change paths since the user may pass something like: intelliperf --project_directory="./" -- ./foo ./my_input_file.

The way we run subprocesses in Intelliperf has changed. Since we are now activating Omniprobe, we must account for the fact that Omniprobe’s exe triggers a nested subprocess.run() call which initializes its own environment, disparate from the environment variables (i.e. PWD) that we initialize in Intelliperf’s capture_subprocess_output() function.

This is why you see that now, if given a working_directory, the function will embed a shell cd command into subprocess_args. This is why I commented out the earlier debug statement, because to be accurate we should print subprocess_args after this conditional.

Omniprobe should respect the environment variables and working dir we set. A command like this intelliperf --project_directory="./" -- ./foo ./my_input_file will break this code.

(Also, I am questioning the whole copy to tmp at the moment but that's for a different PR that I will fix.)

Actually, I take my comment back. This will work fine. I don't understand why Omniprobe will change the working directory though.

src/intelliperf/formulas/bank_conflict.py

mawad-amd

This is looking great. Thanks! I left a few comments.

Signed-off-by: coleramos425 <colramos@amd.com>

…vailible Signed-off-by: coleramos425 <colramos@amd.com>

Signed-off-by: coleramos425 <colramos@amd.com>

coleramos425 · 2025-08-01T19:00:30Z

@mawad-amd feedback has been addressed. Please take another look when you have a chance. Thanks!

Updated logs (based on newest changes):
https://gist.github.com/coleramos425/bbd4dac0ef8d97319ce4cb1ced9a9d69

coleramos425 · 2025-08-18T17:42:37Z

Edit 8/18

Moved containerization material into a separate PR (#142) per request from @mawad-amd

coleramos425 added 5 commits July 12, 2025 19:57

Update omniprobe and nexus repos for https clone

ed732e8

Signed-off-by: coleramos425 <colramos@amd.com>

Remove all old container assets for redesign

156934c

Signed-off-by: coleramos425 <colramos@amd.com>

Redesigned container support. Everything is centerally located and co…

6fc1af2

…nsolidated into a single run.sh script Signed-off-by: coleramos425 <colramos@amd.com>

Update README to reflect new container usage

6bd8ef4

Signed-off-by: coleramos425 <colramos@amd.com>

Adding a VERSION file so containers (eventually to be published) can …

a874cb5

…have some notion of versioning Signed-off-by: coleramos425 <colramos@amd.com>

mawad-amd reviewed Jul 14, 2025

View reviewed changes

containers/intelliperf.Dockerfile Show resolved Hide resolved

mawad-amd reviewed Jul 14, 2025

View reviewed changes

containers/build.sh Outdated Show resolved Hide resolved

coleramos425 and others added 16 commits July 14, 2025 13:37

Enable instrumentation for bankConflict plugin

e2f40fc

Signed-off-by: coleramos425 <colramos@amd.com>

Fix small typo in Dockerfile project install

575b988

Signed-off-by: coleramos425 <colramos@amd.com>

Apply Ruff auto-fixes

b8d19ea

Accept shorthand params on container build scripts

4b1b3f4

Signed-off-by: coleramos425 <colramos@amd.com>

Install omniprobe python packages

55dddbd

Signed-off-by: coleramos425 <colramos@amd.com>

Add get_omniprobe_path() helper for path resolution

e899e09

Signed-off-by: coleramos425 <colramos@amd.com>

Apply Ruff auto-fixes

bc028f2

Fix typo int build script

a437db4

Signed-off-by: coleramos425 <colramos@amd.com>

get_omniprobe_path() needs to be cast to str

24c4436

Signed-off-by: coleramos425 <colramos@amd.com>

Update commit hash in omniprobe and recursivly install python require…

5593c02

…ments Signed-off-by: coleramos425 <colramos@amd.com>

Update optimize pass in bank conflict to use instrumentation results

f7687d8

Signed-off-by: coleramos425 <colramos@amd.com>

Uncomment instruemented kernel name fix for omniprobe kernel filter

f0a5add

Signed-off-by: coleramos425 <colramos@amd.com>

Correctly index instrumentation results array to consider first eleme…

0858f1e

…nt (first kernel) Signed-off-by: coleramos425 <colramos@amd.com>

Modify performance validation pass to account for kernel name from om…

40df901

…niprobe report with clone suffix Signed-off-by: coleramos425 <colramos@amd.com>

Omniprobe uses run.subprocess() with a new env, so PWD isn't set as i…

0f4dbcf

…ntended. Must modify shell cmd to cd to working_directory when specified Signed-off-by: coleramos425 <colramos@amd.com>

Must explicitly use string notation for kernel filters in subprocess …

c3e6f36

…cmds with new implementation Signed-off-by: coleramos425 <colramos@amd.com>

coleramos425 force-pushed the colramos/omniprobe-parser branch from b71a643 to 76c71f3 Compare July 30, 2025 16:54

coleramos425 added 3 commits July 30, 2025 19:52

Adding an omniprobe_path argument to build_examples script

d4cb85b

Signed-off-by: coleramos425 <colramos@amd.com>

Update duplicate build_examples script inside of the examples subdir

fdcb955

Signed-off-by: coleramos425 <colramos@amd.com>

Update container setup instructions in CONTRIBUTING.md

8890e17

Signed-off-by: coleramos425 <colramos@amd.com>

coleramos425 marked this pull request as ready for review July 30, 2025 22:47

coleramos425 requested a review from mawad-amd July 30, 2025 22:47

mawad-amd reviewed Jul 30, 2025

View reviewed changes

external/guided-tuning/bin/gt Show resolved Hide resolved

mawad-amd reviewed Jul 30, 2025

View reviewed changes

src/intelliperf/formulas/bank_conflict.py Outdated Show resolved Hide resolved

mawad-amd reviewed Jul 30, 2025

View reviewed changes

src/intelliperf/formulas/bank_conflict.py Outdated Show resolved Hide resolved

mawad-amd reviewed Jul 30, 2025

View reviewed changes

coleramos425 force-pushed the colramos/omniprobe-parser branch from b0c1697 to f1bb741 Compare July 31, 2025 20:27

coleramos425 and others added 7 commits July 31, 2025 20:33

ruff prelinter should ignore guided-tuning subdir

a36abed

Signed-off-by: coleramos425 <colramos@amd.com>

Restore old optimize_pass() implementation in case omniprobe is not a…

b279f38

…vailible Signed-off-by: coleramos425 <colramos@amd.com>

Comply with linter

5297ecb

Signed-off-by: coleramos425 <colramos@amd.com>

Apply Ruff auto-fixes

82b5c9d

First attempt at adding line numbers into prompt

2e0347c

Signed-off-by: coleramos425 <colramos@amd.com>

Fixing small typo in line number parsing

509b8d0

Signed-off-by: coleramos425 <colramos@amd.com>

Merge branch 'main' into colramos/omniprobe-parser

340f186

coleramos425 changed the title ~~Omniprobe parser + container updates~~ Omniprobe parser for bank conflicts Aug 18, 2025

Merge branch 'main' into colramos/omniprobe-parser

0f778d4

mawad-amd force-pushed the colramos/omniprobe-parser branch from 704c618 to 0f778d4 Compare November 8, 2025 07:32

mawad-amd force-pushed the main branch from 30d1ad0 to a8ffd40 Compare November 8, 2025 07:37

Omniprobe parser for bank conflicts #123

Are you sure you want to change the base?

Omniprobe parser for bank conflicts #123

Uh oh!

Conversation

coleramos425 commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Major changes:

Example:

Uh oh!

mawad-amd commented Jul 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mawad-amd Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

coleramos425 Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

mawad-amd Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

mawad-amd Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coleramos425 Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

mawad-amd Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

mawad-amd Aug 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mawad-amd left a comment

Choose a reason for hiding this comment

Uh oh!

coleramos425 commented Aug 1, 2025

Uh oh!

coleramos425 commented Aug 18, 2025

Edit 8/18

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coleramos425 commented Jul 12, 2025 •

edited

Loading

mawad-amd Jul 30, 2025 •

edited

Loading