You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/08/30 11:47:23 UTC
[GitHub] [tvm] sisleyli opened a new pull request, #12646: [Analysis] Add dump op by relay var number for analysis
sisleyli opened a new pull request, #12646:
URL: https://github.com/apache/tvm/pull/12646
Hi,
When I actually develop operators for some models, I often need to do some verification.
And when I encounter a situation where the input and output values of the entire model do not match the golden values, I often need to dump some intermediate nodes to confirm which node the problem starts from.
As we know, we can easily print Relay IR by `print(mod["main"])`,
Relay IR represents each node as "%xx %10 %20", I feel comfortable if we can dump nodes with "%xx" as a parameter.
So I develop a pass that can dump op by `dump_op_by_num(mod,xx)`.
Because we generally use dump OP for analysis purposes, I set it as an `relay.analysis` API.
What do you think? Do we need such an API?
I am looking forward to your reply. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232321802
I think such pass is useful indeed, but I don't have a good idea on how to identify the intermediate node that will become the output node in the extracted subgraph.
Your approach that uses an integer and incrementing the count as we go through ops is probably ok for chain-like models, but for more general models where there is no clear ordering on nodes, this would not work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232571435
ok, I agree that using the order from print works in practice.
I don't like the name "DumpOPByName`. How about this signature: `ExtractIntermediateExpr(mod, expr_id)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a diff in pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #12646:
URL: https://github.com/apache/tvm/pull/12646#discussion_r960171729
##########
src/relay/analysis/extract_intermediate_expr.cc:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file extract_intermediate_expr.cc
+ * \brief Used for extracting Relay Expr
+ by the expression ID of the main function
+ that we can see in `print(mod["main"])`.
+ */
+#include <tvm/node/structural_hash.h>
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+
+namespace tvm {
+namespace relay {
+
+class ExtractIntermediateExprWrapper : private MixedModeVisitor {
+ public:
+ explicit ExtractIntermediateExprWrapper(const IRModule& mod, const int expr_id)
+ : mod_(mod), target_expr_id_(expr_id), counter_(0) {}
+
+ IRModule Extract() {
+ VisitExpr(this->mod_->Lookup("main"));
+
+ // ensure the target op num we want to dump is valid.
+ ICHECK(target_expr_id_ >= 0 && target_expr_id_ < counter_);
+
+ return IRModule::FromExpr(target_op_, {});
+ }
+
+ private:
+ using MixedModeVisitor::VisitExpr_;
+
+ const IRModule mod_;
+ /*! \brief the OP Number that we want to dump. */
Review Comment:
OP Number -> expression ID
dump -> extract
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Relay] Extract intermediate node by its expression ID
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1233763281
Hi @masahi, CI shows that `python: i386 2 of 5` fail. But I just modified some comments compared to the previous commit. I don't know where the problem is. Could you please help me? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a diff in pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #12646:
URL: https://github.com/apache/tvm/pull/12646#discussion_r960171600
##########
src/relay/analysis/extract_intermediate_expr.cc:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file extract_intermediate_expr.cc
+ * \brief Used for extracting Relay Expr
+ by the expression ID of the main function
+ that we can see in `print(mod["main"])`.
+ */
+#include <tvm/node/structural_hash.h>
+#include <tvm/relay/analysis.h>
+#include <tvm/relay/expr.h>
+#include <tvm/relay/expr_functor.h>
+
+namespace tvm {
+namespace relay {
+
+class ExtractIntermediateExprWrapper : private MixedModeVisitor {
+ public:
+ explicit ExtractIntermediateExprWrapper(const IRModule& mod, const int expr_id)
+ : mod_(mod), target_expr_id_(expr_id), counter_(0) {}
+
+ IRModule Extract() {
+ VisitExpr(this->mod_->Lookup("main"));
+
+ // ensure the target op num we want to dump is valid.
Review Comment:
num -> id
dump -> extract
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232577473
btw, have you used `debug_executor`? I heard that it can also dump output tensors of intermediate nodes. Your use case may be covered by `debug_executor` as well. cc @areusch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a diff in pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #12646:
URL: https://github.com/apache/tvm/pull/12646#discussion_r960171252
##########
python/tvm/relay/analysis/analysis.py:
##########
@@ -431,3 +431,41 @@ def get_calibration_data(mod, data):
calib_data[gvar] = value
return calib_data
+
+
+def extract_intermdeiate_expr(mod, expr_id):
+ """Extract Relay Expr by the expression ID
Review Comment:
the -> its
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a diff in pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #12646:
URL: https://github.com/apache/tvm/pull/12646#discussion_r960150595
##########
python/tvm/relay/analysis/analysis.py:
##########
@@ -431,3 +431,23 @@ def get_calibration_data(mod, data):
calib_data[gvar] = value
return calib_data
+
+
+def extract_intermdeiate_expr(mod, expr_id):
+ """Extract Relay Expr by the local variable number
+
+ This function is used for extracting Relay Expr
+ by the local variable number of the main function
+ that we can see in `print(mod["main"])`.
+
+ Parameters
+ ----------
+ mod : tvm.IRModule
+
+ expr_id : the Expr Number that we want to extract
Review Comment:
Number -> ID
##########
python/tvm/relay/analysis/analysis.py:
##########
@@ -431,3 +431,23 @@ def get_calibration_data(mod, data):
calib_data[gvar] = value
return calib_data
+
+
+def extract_intermdeiate_expr(mod, expr_id):
+ """Extract Relay Expr by the local variable number
+
+ This function is used for extracting Relay Expr
+ by the local variable number of the main function
+ that we can see in `print(mod["main"])`.
Review Comment:
It would be nice to have an example here. `local variable number` is not a word in TVM, we should just refer to them as expression ID or something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1231561732
Hi @masahi ! Do you think we need such a analysis pass? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Relay] Extract intermediate node by its expression ID
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1234080282
Hi @masahi , I see that CI has passed. is it able to be merged, or need more work? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a diff in pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on code in PR #12646:
URL: https://github.com/apache/tvm/pull/12646#discussion_r960171148
##########
python/tvm/relay/analysis/analysis.py:
##########
@@ -431,3 +431,41 @@ def get_calibration_data(mod, data):
calib_data[gvar] = value
return calib_data
+
+
+def extract_intermdeiate_expr(mod, expr_id):
+ """Extract Relay Expr by the expression ID
+
+ This function is used for extracting Relay Expr
+ by the expression ID of the main function
+ that we can see in `print(mod["main"])`.
+
+ Parameters
+ ----------
+ mod : tvm.IRModule
+
+ expr_id : the Expr ID that we want to extract
+
+ Returns
+ -------
+ ret : Extracted IRModule
+
+ Examples
+ --------
+ .. code-block:: python
+
+ # Supposed our module is printed like this:
Review Comment:
Suppose
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
areusch commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1233386563
ah yeah this is related to what we [discussed](https://discuss.tvm.apache.org/t/an-enabling-framework-for-int8-quantization/13269/7?u=areusch) a few weeks at community meeting. There hasn't been a follow-on edge ID proposal yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232375360
Hi masashi, thanks for your reply.
The more general models you mean are RNN/LSTM models, or models like below?
```
0
/ | \
/ | \
1 2 3
```
The integer ID that I request is the number that `RelayPrinter` alloc to every node.(which we can see this id by `print(mod["main"])` easily). The original intention of this method is to use the printed relay IR information to dump the specified op. So once the order of printing is determined, I think we just need to make sure that the integer ID specified by this method is consistent with the printed variable id.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232694440
Hi masahi, when I use `debug_executor`, There are many nodes called `tvmgen_default_fused_xxxxx`, it seems that the operators that debugger dump have already been fused(I didn't use any pass). Therefore it is not easy for me to dump the output of a specific op in Relay level.
I'm not sure if there are some params to control this fusion. cc @areusch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1233670030
@sisleyli More doc comments, but otherwise looks good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi merged pull request #12646: [Relay] Extract intermediate node by its expression ID
Posted by GitBox <gi...@apache.org>.
masahi merged PR #12646:
URL: https://github.com/apache/tvm/pull/12646
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232966801
I see, CI has passed now, please review again when you have free time. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Relay] Extract intermediate node by its expression ID
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1233784777
@tvm-bot rerun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
masahi commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232702666
I see, that makes sense (`debug_executor` only allows outputs from post fusion).
Please rebase against `main`, otherwise CI fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on pull request #12646: [Analysis] Add dump op by relay var number for analysis
Posted by GitBox <gi...@apache.org>.
sisleyli commented on PR #12646:
URL: https://github.com/apache/tvm/pull/12646#issuecomment-1232648593
> expr_id
I agree `ExtractIntermediateExpr(mod, expr_id)` is better, I have modified my code.
I have used `debug_executor`. Do you mean `m.debug_get_output(node,out=tvm_output)`?
I remembered the param `node` was not in Relay level, I will check later. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org