In et_generator, change 'comm_size' attr from uint64 to int64 #150
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes
et_generator
useint64
, instead ofuint64
for 'comm_size' when creating collective nodes.et_feeder
parsescomm_size
as aint64
datatype(et_feeder.cpp:28). With the current strong datatype conversion, using auint64
value would makeattr.int64_val()
parse this value as 0 (appendix 1)Also added
static_cast
into uint64_t forcomm_size
, which #18 seems to have missed.(@TaekyungHeo , please confirm if the lack of static_cast was intentional)
Test Plan
Created & ran the following test locally, to verify that et_generator using
uint64
causes comm_size to be parsed as 0.I did not add this test into the PR, as this is only used to verify the problem statement, not any of our code.
Generate a test ET using the following code
To generate the following trace
To be tested with the following test
Additional Notes
Appendix 1)
From
et_def.pb.h
: