-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix counter collection inconsistency with rocprofv3 #589
base: develop
Are you sure you want to change the base?
Conversation
* Fix post analysis gui in standalone binary * Add post analysis gui assets and required server libraries for GUI server and web page * Add port forwarding to docker test compose * Update README me to use `docker compose up` instead of `docker compose run` to run containers with port forwarding and to leverage other functionalities of docker compose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some minor comments
src/rocprof_compute_soc/soc_base.py
Outdated
def using_v3(): | ||
return "ROCPROF" in os.environ.keys() and os.environ["ROCPROF"] == "rocprofv3" | ||
return "ROCPROF" in os.environ.keys() and "rocprofv3" in os.environ["ROCPROF"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious as to why does this hack do substring match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it now, ROCPROF env. var. will contain the path to rocprofv3, in that case should we use os.environ["ROCPROF"].endswith("rocprofv3")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -209,7 +209,8 @@ def create_single_df_pmc(raw_data_dir, node_name, kernel_verbose, verbose): | |||
dfs.append(tmp_df) | |||
coll_levels.append(f[:-4]) | |||
|
|||
final_df = pd.concat(dfs, keys=coll_levels, axis=1, copy=False) | |||
# TODO: double check the case if all tmp_df.shape[0] are not on the same page | |||
final_df = pd.concat(dfs, keys=coll_levels, axis=1, join="inner", copy=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in the meeting, inner join based on index is the only option we have since kernel name and dispatch ids might be non-deterministic
For some case, we noticed collected total row numbers are not the same across all perfmon csv files:
pmc_perf.csv, SQ_IFETCH_LEVEL.csv, SQ_INST_LEVEL_LDS.csv, SQ_INST_LEVEL_SMEM.csv, SQ_INST_LEVEL_VMEM.csv.
The quick solution is: inner join if collected csv are not on the same page.
We might need to dig into and check the root reason.