-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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] Add ConstantNode to IsAtomic #5457
Conversation
@zhiics how big is a constant? is it ok to copy them around, or is any tensor literal possible in there? |
@MarisaKirisame Good question, I am not sure about how large it is. @kevinthesun had a failed case. Maybe he can give a better answer. |
@zhiics I am not asking about the specific case here. I am asking that can they be big. |
I tooked a look at the discuss, and it seems like the way to fix it is to just call ANF from Fold-Constant before going into the interpreter pass. can you do that? |
@MarisaKirisame The current use cases which can trigger is this issue are mainly from TensorFlow OD models. For those models, constant tensors which can be feed into fold_const pass are mostly small. |
@MarisaKirisame Sorry for the late response. Yeah, transforming it to ANF should work as well and seems like better fix. Let me try it. BTW, this would also mean that that we may have to execute ANF multiple times for the program executing through VM because we do constant folding after ANF in vm optimization. In general, I don't think there wouldn't be any problem. Do you see any possible issue? |
@zhiics the ANF pass is pretty fast, and it seems like if you sum the size of the graph of all call to ANF, the size will be smaller then the original graph. so i dont think it is bad. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @zhiics @MarisaKirisame |
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* [Fix] Add ConstantNode to IsAtomic (#5457) * add constantnode to atomic * Add ToANormalForm to FoldConstant * fix test
* add constantnode to atomic * Add ToANormalForm to FoldConstant
* add constantnode to atomic * Add ToANormalForm to FoldConstant
The problem is described here: https://discuss.tvm.ai/t/relay-pass-feature-check-fail-during-fold-constant-pass/6498
@MarisaKirisame Can you take a look and see if the change makes sense?
cc @kevinthesun