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] Change Default "opt_level" of Sequential from 2 to 0 #8634

Merged
merged 1 commit into from
Aug 4, 2021

Conversation

Johnson9009
Copy link
Contributor

@Johnson9009 Johnson9009 commented Aug 3, 2021

Current default "opt_level" of Sequential is 2, this value will lead to bug when sequential nested.
The below test case can't pass, through the opt_level of pass SimplifyExpr is 0, but the opt_level of wrapper sequential is 2, so the pass SimplifyExpr will not be run.

def test_nested_sequential_with_scoping():
    def before():
        x = relay.var("x", shape=(1, 16, 16, 16), dtype="float32")
        w = relay.var("w", shape=(32, 16, 3, 3), dtype="float32")
        y = relay.nn.conv2d(x, w, padding=(1, 1))
        y = relay.reshape(y, newshape=(1, 16, -1))
        y = relay.reshape(y, newshape=(4, 8, -1, 16))
        y = relay.reverse_reshape(y, newshape=(32, 0, -1))
        return tvm.IRModule.from_expr(y)

    def expected():
        x = relay.var("x", shape=(1, 16, 16, 16), dtype="float32")
        w = relay.var("w", shape=(32, 16, 3, 3), dtype="float32")
        y = relay.nn.conv2d(x, w, padding=(1, 1))
        y = relay.reshape(y, newshape=(32, 16, 16))
        return tvm.IRModule.from_expr(y)

    z = before()
    passes = [
        tvm.transform.Sequential([relay.transform.SimplifyExpr()]),
    ]
    with tvm.transform.PassContext(opt_level=1):
        zz = tvm.transform.Sequential(passes)(z)

    expected = relay.transform.InferType()(expected())
    assert tvm.ir.structural_equal(zz, expected)

Actually from the comments of class Sequential, it seems we think the default value of this opt_level should be 0, but the code is not changed.

opt_level : Optional[int]
The optimization level of this sequential pass.
The opt_level of a default sequential pass is set to 0.
Note that some of the passes within the Sequantial may still not be executed
if their opt_level is higher than the provided opt_level.

@tqchen @zhiics

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

The CI gets timeout. Not sure if the slowdown was due to opt_level=0 or just a CI issue. Maybe simply re-trigger CI and see how it goes.

@masahi masahi merged commit b9204cd into apache:main Aug 4, 2021
@Johnson9009 Johnson9009 deleted the pass_manager branch August 4, 2021 02:12
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