Skip to content

[Relay][AST] Add WithFields for Constant and GlobalVar #10929

Closed
mikepapadim wants to merge 2 commits into
apache:mainfrom
mikepapadim:extend_withfields
Closed

[Relay][AST] Add WithFields for Constant and GlobalVar #10929
mikepapadim wants to merge 2 commits into
apache:mainfrom
mikepapadim:extend_withfields

Conversation

@mikepapadim

@mikepapadim mikepapadim commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

Co-authored-by: Mark Shields mbs@octoml.ai

This PR adds WithFields for Constant and GlobalVar.
It is an addition of the original work described in #9569 and #9533 which provided support for the reminder of the relay nodes.

@mbs-octoml @mbaret

@mbaret

mbaret commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

I think this PR needs some more detail on what it's adding and why (is this part of a larger issue/RFC?) It should also include some simple tests to demonstrate the functionality.

Co-authored-by: Mark Shields <mbs@octoml.ai>
@mikepapadim

Copy link
Copy Markdown
Contributor Author

PTAL this one now introduces cpp tests for the WithFields

Comment on lines +36 to +37
ICHECK(func_init->span == func_cp->span);
ICHECK(func_init->checked_type_ == func_cp->checked_type_);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to explicitly set these fields so that we know they're being preserved by WithFields (same for next test).

@areusch areusch added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it and removed needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it labels Oct 19, 2022
@tqchen tqchen closed this Dec 10, 2024
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.

4 participants