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

[Relay] Extract intermediate node by its expression ID #12646

Merged
merged 1 commit into from Sep 1, 2022

Conversation

sisleyli
Copy link
Contributor

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!

@sisleyli
Copy link
Contributor Author

sisleyli commented Aug 30, 2022

Hi @masahi ! Do you think we need such an analysis pass? Thank you!

@masahi
Copy link
Member

masahi commented Aug 31, 2022

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 requests an integer ID from users 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.

@sisleyli
Copy link
Contributor Author

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.

@masahi
Copy link
Member

masahi commented Aug 31, 2022

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)

@masahi
Copy link
Member

masahi commented Aug 31, 2022

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

@sisleyli
Copy link
Contributor Author

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!

@sisleyli
Copy link
Contributor Author

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

@masahi
Copy link
Member

masahi commented Aug 31, 2022

I see, that makes sense (that debug_executor only allows outputs from post fusion).

Please rebase against main, otherwise CI fails.

@sisleyli
Copy link
Contributor Author

I see, CI has passed now, please review again when you have free time. Thank you!

@areusch
Copy link
Contributor

areusch commented Aug 31, 2022

ah yeah this is related to what we discussed a few weeks at community meeting. There hasn't been a follow-on edge ID proposal yet.


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"])`.
Copy link
Member

Choose a reason for hiding this comment

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

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.

----------
mod : tvm.IRModule

expr_id : the Expr Number that we want to extract
Copy link
Member

Choose a reason for hiding this comment

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

Number -> ID

--------
.. code-block:: python

# Supposed our module is printed like this:
Copy link
Member

Choose a reason for hiding this comment

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

Suppose



def extract_intermdeiate_expr(mod, expr_id):
"""Extract Relay Expr by the expression ID
Copy link
Member

Choose a reason for hiding this comment

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

the -> its

IRModule Extract() {
VisitExpr(this->mod_->Lookup("main"));

// ensure the target op num we want to dump is valid.
Copy link
Member

Choose a reason for hiding this comment

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

num -> id

dump -> extract

using MixedModeVisitor::VisitExpr_;

const IRModule mod_;
/*! \brief the OP Number that we want to dump. */
Copy link
Member

Choose a reason for hiding this comment

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

OP Number -> expression ID

dump -> extract

@masahi
Copy link
Member

masahi commented Sep 1, 2022

@sisleyli More doc comments, but otherwise looks good

@masahi masahi changed the title [Analysis] Add dump op by relay var number for analysis [Relay] Extract intermediate node by its expression ID Sep 1, 2022
@sisleyli
Copy link
Contributor Author

sisleyli commented Sep 1, 2022

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!

@masahi
Copy link
Member

masahi commented Sep 1, 2022

@tvm-bot rerun

@sisleyli
Copy link
Contributor Author

sisleyli commented Sep 1, 2022

Hi @masahi , I see that CI has passed. is it able to be merged, or need more work? Thank you!

@masahi masahi merged commit 38ba8c0 into apache:main Sep 1, 2022
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
[Relay] Extract Intermediate Expr by relay expr ID for analysis

modify doc comments

Co-authored-by: Bin Li <binli1@amd.com>
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.

None yet

3 participants