Skip to content

Conversation

@hxu296
Copy link

@hxu296 hxu296 commented Feb 4, 2025

Summary

Updates the handling of optional attributes for ET feeder nodes. Previously, default values for fields like num_ops and tensor_size were uninitialized, potentially returning random values. The changes:

  • Introduce std::optional for fields that may not have a value.
  • Ensure getters return a default value when no value is present.
  • Add an INVALID_COMM_TYPE to the protobuf CollectiveCommType enum to represent a default communication type.

These updates improve the robustness of the code by preventing uninitialized data from being returned.

this->is_cpu_op_ = static_cast<bool>(attr.bool_val());
} else if (attr_name == "num_ops") {
this->num_ops_ = static_cast<uint64_t>(attr.int64_val());
} else if (attr_name == "tensor_loc_") {
Copy link
Collaborator

@JoongunPark JoongunPark Feb 4, 2025

Choose a reason for hiding this comment

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

QQ: is the attribute name "tensor_loc_" not "tensor_loc"?
What upstream tool uses that name? STG?
I think Chakra converter does not use that attribute name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants