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

[Diagnostics][Relay][InferType] Refactor InferType to work on whole module, and use new diagnostics. #6274

Merged
merged 40 commits into from
Oct 9, 2020

Conversation

jroesch
Copy link
Member

@jroesch jroesch commented Aug 13, 2020

This is now ready for full review. This has become a huge PR where I split the coupling between the type checker and the module enabling more flexible use of the module and type checker. Unfortunately people have heavily relied on implicit type checking occurring each time we add a function. This functionality is not longer supported and examples of fixing it can be found all over the test cases.

include/tvm/ir/diagnostic.h Outdated Show resolved Hide resolved
include/tvm/ir/diagnostic.h Outdated Show resolved Hide resolved
include/tvm/ir/diagnostic.h Outdated Show resolved Hide resolved
include/tvm/ir/diagnostic.h Show resolved Hide resolved
include/tvm/ir/diagnostic.h Show resolved Hide resolved
include/tvm/ir/diagnostic.h Outdated Show resolved Hide resolved
@junrushao
Copy link
Member

May be out of the scope of this PR, but shall we also consider make type inference non-recursive some time?

src/ir/module.cc Show resolved Hide resolved
src/parser/parser.cc Outdated Show resolved Hide resolved
src/relay/transforms/type_infer.cc Outdated Show resolved Hide resolved
tests/python/relay/test_ir_parser.py Outdated Show resolved Hide resolved
@jroesch jroesch marked this pull request as ready for review September 4, 2020 02:10
src/ir/diagnostic.cc Outdated Show resolved Hide resolved
src/ir/diagnostic.cc Outdated Show resolved Hide resolved
src/ir/diagnostic.cc Outdated Show resolved Hide resolved
src/ir/diagnostic.cc Outdated Show resolved Hide resolved

def _add(self, var, val, update=False):
def _add(self, var, val, update=True):
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove update/add in future PR.

@@ -213,13 +213,15 @@ def optimize(self):
"""
seq = tvm.transform.Sequential(
[
# tvm.parser.AnnotateSpans(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we provide a flag for spans here?

@@ -1512,3 +1552,6 @@ def load_prelude(self):
]:
tensor_array_ops = TensorArrayOps(self, dtype)
tensor_array_ops.register()

# Renamer doesn't properly deal with constructors, etc
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this is true anymore.

src/parser/parser.cc Outdated Show resolved Hide resolved
src/parser/parser.cc Outdated Show resolved Hide resolved
@tmoreau89 tmoreau89 merged commit 98c2096 into apache:master Oct 9, 2020
@tmoreau89
Copy link
Contributor

Thank you @jroesch @mbrookhart @rkimball the PR has been merged.

TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 13, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 14, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 16, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 16, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (neo-ai#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Oct 19, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Oct 19, 2020
…odule, and use new diagnostics. (apache#6274)

* Refactor the type checker to use diagnostics

Although this patch is very large and seemingly disjoint the
fixes are required to get it working for the entire stack.
I started with first changing InferType to use the diagnostics,
these weren't yet in the pass manager so this required changes
to module and module pass. InferType wasn't actually written
correctly as a pass requring refactoring there, then in order
to add spans to AST it required turning on AnnotateSpans which
in term required changes to the parser, and module to make
it possible to use the errors. These changes to parse and module
required changes to diagnostics and InferType. Althought seemingly
disconnected there are hidden cycles between the components which
require simultaneous change in order to remove the old error
reporting.

A huge change due to this patch is that the module no longer
implicitly type checks functions which are added.

* Apply suggestions from code review

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>

* Apply suggestions from code review

Co-authored-by: Tristan Konolige <[email protected]>

* Clean up parser

* CR feedback

* Apply Bobs suggestions

* Fix up Python interface for diagnostics

* Fix test_ir_parser and formatting

* Fix cpplint

* Fix lint

* Fix format

* More lint

* Fix format

* Kill dead doc comment

* Fix documentation comment

* Rebase fixups

* Add docs for type.h

* Fix parser.cc

* Fix unittests

* Fix black

* Skip previously typechecked functions

* fix ACL

* Fix numerous issues

* Add repr method

* Fix issue with Pytest, I am ready to cry

* Fix the rest of tests

* Kill dead code

* Fix dignostic tests

* Fix more tests

* fix more tests (#11)

* Fix diagnostic.py deinit bug

* Fix deinit issue

* Format

* Tweak disabling of override

* Format

* Fix BYOC

* Fix TensorArray stuff

* Fix PyTorch

* Format

* Format

Co-authored-by: Robert Kimball <[email protected]>
Co-authored-by: Junru Shao <[email protected]>
Co-authored-by: Tristan Konolige <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Zhi <[email protected]>
@t-vi t-vi mentioned this pull request Jan 29, 2021
@jroesch jroesch deleted the diag_infer_type branch February 4, 2021 04:42
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.

9 participants