Skip to content
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

Timeline: Add MEMCPY_IN_FUSION_BUFFER event for Reducescatter #3808

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

maxhgerlach
Copy link
Collaborator

Checklist before submitting

  • Did you read the contributor guide?
  • Did you update the docs?
  • Did you write any tests to validate this change?
  • Did you update the CHANGELOG, if this change affects users?

Description

The MEMCPY_IN_FUSION_BUFFER event accidentally wasn't recorded for NCCLReducescatter, which made Horovod timelines a bit confusing to read.

Review process to land

  1. All tests and other checks must succeed.
  2. At least one member of the technical steering committee must review and approve.
  3. If any member of the technical steering committee requests changes, they must be addressed.

Signed-off-by: Max H. Gerlach <git@maxgerlach.de>
@maxhgerlach maxhgerlach requested a review from romerojosh January 6, 2023 10:26
@maxhgerlach
Copy link
Collaborator Author

Build failures with macOS are unrelated to this change (#3809).

@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Unit Test Results

  1 155 files  ±0    1 155 suites  ±0   13h 21m 10s ⏱️ + 43m 24s
     841 tests ±0       788 ✔️ ±0       53 💤 ±0  0 ±0 
23 809 runs  ±0  17 023 ✔️ ±0  6 786 💤 ±0  0 ±0 

Results for commit 0d6d6ff. ± Comparison against base commit 4ad1d43.

@github-actions
Copy link

github-actions bot commented Jan 6, 2023

Unit Test Results (with flaky tests)

  1 220 files  +  39    1 220 suites  +39   13h 44m 51s ⏱️ + 57m 38s
     841 tests ±    0       787 ✔️ ±    0       53 💤 ±  0  1 ±0 
24 654 runs  +507  17 828 ✔️ +483  6 821 💤 +21  5 +3 

For more details on these failures, see this check.

Results for commit 0d6d6ff. ± Comparison against base commit 4ad1d43.

Copy link
Collaborator

@romerojosh romerojosh left a comment

Choose a reason for hiding this comment

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

LGTM!

@maxhgerlach maxhgerlach merged commit 2f52345 into master Jan 6, 2023
@maxhgerlach maxhgerlach deleted the fix-reducescatter-timeline-memcpyin branch January 6, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants