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

Support Any type in AttributeProto #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ajbalogh
Copy link

@ajbalogh ajbalogh commented Mar 28, 2024

Summary

This PR adds the Any type to the AttributeProto message as a single any_val and repeated any_list.

The Any type in Any.proto is a standard Protobuf definition included with the Protobuf installation.

The rationale behind this is that a user wants to use complex protobuf messages for metrics and configuration and add them to individual et_def.Node messages.

The any_val field in AttributeProto allows a user to easily serialize/deserialize messages into a single field while preserving the structure of the message and associated internal documentation and options.

@ajbalogh ajbalogh requested a review from a team as a code owner March 28, 2024 13:58
Copy link

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@TaekyungHeo
Copy link
Contributor

TaekyungHeo commented Apr 1, 2024

Thank you for your contribution, Andy!

  • Could you please share the motivation behind introducing the new fields, Any and AnyList?
  • Is it necessary to import google/protobuf/any.proto? If these are not native data types supported by protobuf, could you explain the rationale for including them?

@TaekyungHeo TaekyungHeo changed the title support Any type in AttributeProto Support Any type in AttributeProto Apr 1, 2024
@ajbalogh
Copy link
Author

ajbalogh commented Apr 1, 2024

@TaekyungHeo I've updated the summary to include the rationale behind the any type.

@srinivas212
Copy link
Contributor

@ajbalogh - repeating a question @TaekyungHeo already asked... if Any is a native type, why do we need to import the type?

@ajbalogh
Copy link
Author

ajbalogh commented Apr 5, 2024

@srinivas212 @TaekyungHeo - Any isn't a native type but a standard protobuf definition that is included with the protobuf installation and as result requires it to be imported.

@TaekyungHeo
Copy link
Contributor

@srinivas212 , I think @ajbalogh 's point makes sense.

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.

3 participants