Skip to content

Conversation

@lithomas1
Copy link
Collaborator

We were reading extraneous NVTX events (e.g. domain creation).
Modified to only read range events.

In _match_caller_callee, we should always be assigning to the list using integer indices, instead of the dataframe index.
(This is wrong when the dataframe index is not a range index going from 0...n, which happens after we select a subset of rows for example)

@lithomas1
Copy link
Collaborator Author

OK, I think I have a bug where this doesn't work with a filtered df.
In my previous example, I only have 1 process so this doesn't show up.

Solution is probably to rewrite this using loc.

@jhdavis8 jhdavis8 self-requested a review April 17, 2025 16:54
@jhdavis8 jhdavis8 self-requested a review April 17, 2025 16:55
assert np.isclose(norm.loc[61]["MPI_Comm_size"], 0.0)
assert np.isclose(norm.loc[61]["MPI_Comm_rank"], 0.0)
assert np.isclose(norm.loc[61]["MPI_Finalize"], 0.01614835)
assert_allclose(norm.loc[0]["int main(int, char**)"], 0.00299437, rtol=1e-05)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert_allclose is the cleaner/more recommended way for comparing numpy arrays.

I had to adjust the tolerances since assert_allclose uses stricter tolerances than isclose for some of them.
(I'm not sure how the expected numbers were acquired, but I'm guessing that some of them were rounded which would make us off by a little here).

Copy link

@jhdavis8 jhdavis8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@lithomas1 lithomas1 closed this Apr 18, 2025
@lithomas1 lithomas1 reopened this Apr 18, 2025
@lithomas1 lithomas1 merged commit a83a5f1 into hpcgroup:develop Apr 18, 2025
16 checks passed
@lithomas1 lithomas1 deleted the gpu-fixes branch April 18, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants