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

Fix inconsistent IR when printing R.nn.pad operator #17567

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

Conversation

parsifal-47
Copy link
Contributor

Hi All,
This is a fix for #17483
the inconsistency caused by pad_width and pad_value being second argument, this PR moves pad_value to the attributes.
Let me know if that is not a proper way to resolve the issue, additionally I am not sure if there is a way to make a test for show, I would appreciate references in this case.

Thank you!

@yongwww
Copy link
Member

yongwww commented Dec 29, 2024

@tvm-bot rerun

3 similar comments
@yongwww
Copy link
Member

yongwww commented Dec 30, 2024

@tvm-bot rerun

@parsifal-47
Copy link
Contributor Author

@tvm-bot rerun

@Cookiee235
Copy link
Contributor

@tvm-bot rerun

@parsifal-47
Copy link
Contributor Author

@Cookiee235 I do not understand the workflow completely, but it looks like some approval is needed in order to run the rest of the PR checks

@yongwww
Copy link
Member

yongwww commented Jan 18, 2025

@tvm-bot rerun

@parsifal-47 parsifal-47 force-pushed the relax-inconsistent-print branch from 84d74e2 to 9d45cdb Compare January 26, 2025 03:24
@parsifal-47 parsifal-47 force-pushed the relax-inconsistent-print branch from fc4c7c2 to e38971a Compare January 26, 2025 23:30
@parsifal-47
Copy link
Contributor Author

parsifal-47 commented Jan 26, 2025

tests are working now, but when going from tensorflow in tests/python/contrib/test_msc/test_translate_tensorflow.py the constant is not getting folded and remains Var or getting converted to Var for some reason, not sure how to fix that.

I tried adding transforms to AttrCvt with fold_constant, but it looks like the Var appears somewhere after.

@parsifal-47
Copy link
Contributor Author

to elaborate a bit more, originally, when pad_value was passed as positional argument, it was folded automatically, but because pad_width was actually the second positional argument, printer was broken. After I moved it to attributes, automatic folding no longer applies and it looks like legalizer is not a good place to do that because I do not see any other foldings there. Looks like the best place is in conversion from tf because other tests are green, but I tired multiple combinations and haven't found any workable, will take another look in a few days, meanwhile if you have an advice, let me know, thank you!

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