Skip to content

Add SizeNe#4338

Open
miladm wants to merge 1 commit intomasterfrom
add_ne_size_node
Open

Add SizeNe#4338
miladm wants to merge 1 commit intomasterfrom
add_ne_size_node

Conversation

@miladm
Copy link
Collaborator

@miladm miladm commented Dec 15, 2022

No description provided.

@miladm miladm added the dynamism Dynamic Shape Features label Dec 15, 2022
@miladm miladm self-assigned this Dec 15, 2022
return dim_node_0->getDynamicValue() != dim_node_1->getDynamicValue() ? 1 : 0;
}

std::string SizeNe::ToString() const { return "SizeNe"; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you make ToString return "aten::ne_size"? The string has to contain the "aten::ne" otherwise you won't be able to dump the ir of it.

@vanbasten23
Copy link
Collaborator

if possible, can you add some python level unit test?


SizeNe::SizeNe(torch::lazy::Value a, torch::lazy::Value b)
: XlaNode(torch::lazy::OpKind{c10::Symbol::fromQualString("aten::ne")},
{a, b}, xla::ShapeUtil::MakeShape(xla::S64, {}), 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

let' use GetShapeDimensionType, @vanbasten23 when can we update all those s64 in the sizeNode to GetShapeDimensionType?

Copy link
Collaborator

@vanbasten23 vanbasten23 Dec 20, 2022

Choose a reason for hiding this comment

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

Ack. I'm blocked on #4258 to update all return type: I could've done the casting to unblock me but I'd really like to understand where the s32/s64 mismatch comes from before blindly do the casting. I left a comment. Can you take another look @JackCaoG ?

On another thought, I'm not technically blocked by the above PR to update all those s64 in the sizeNode to use GetShapeDimensionType , let me do that today. Thanks for the reminder.

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

Labels

dynamism Dynamic Shape Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants