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

Update message_generated.h to match flatbuffers schema #3816

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

maxhgerlach
Copy link
Collaborator

@maxhgerlach maxhgerlach commented Jan 13, 2023

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

Here I've just run flatc on horovod/common/wire/message_generated.fbs. The generated header file was out of sync regarding one of the ReduceOp enum names (as pointed out in #3812).

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>
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.

Oof, nice catch. That seems to have been due to a last minute commit from my PR that added the new ops where I changed the op name from Prod to Product. Seems the flatbuffers implementation doesn't care about this, since the ordering (and index assignment) is consistent. Good to make consistent anyway!

@github-actions
Copy link

Unit Test Results

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

Results for commit dcd4423. ± Comparison against base commit bfe96f8.

@github-actions
Copy link

Unit Test Results (with flaky tests)

  1 305 files  +  10    1 305 suites  +10   13h 43m 47s ⏱️ + 12m 46s
     841 tests ±    0       788 ✔️ +  1       53 💤 ±    0  0  - 1 
27 083 runs  +314  18 919 ✔️ +18  8 164 💤 +298  0  - 2 

Results for commit dcd4423. ± Comparison against base commit bfe96f8.

Copy link
Collaborator

@EnricoMi EnricoMi 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 66769df into master Jan 14, 2023
@maxhgerlach maxhgerlach deleted the update-message_generated-flatbuffers branch January 14, 2023 12:23
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.

None yet

3 participants