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] parser/pretty printer roundtripping #3536

Merged
merged 3 commits into from
Jul 18, 2019

Conversation

MarisaKirisame
Copy link
Contributor

@MarisaKirisame MarisaKirisame commented Jul 12, 2019

This PR add support for roundtripping relay code from/to string.
Right now, it support roundtripping all the defined network, and I think this is a good time to wrap up and merge.
A few things to discuss:
Doc does not accept string input with newline/tab any more, as it will interfere with the layout algorithm that will land in the future.
The printing strategy is ugly for anfed code. It will print a variable, then graph that variable.
Similarly, there should be inline ability for simple expression such as a + b + c, instead of always allocate graph var.
FuncOp is highly redundant. Can we get this information with tvm?
I copied the josn pretty printer so it use Doc Style.
since ppl do assert alpha_equal all the time, I build it into alpha_equal to give better error message.

Future Work:
0: pattern matching/adt/fatal/ref for both pretty printing/parser
1: doc.h do not have a layout engine - right now it will emit the same string regardless of screen width/remaining space
2: indentation is off - nested let look incredibly broken
3: every tree - style program will still make a graph var for everything, so it is impossible to display code that resemble sml
4: deprecate the old json pretty printer?
5: src/error.cc often do not display error message for unknown reason
6: more principle solution for dealing with literal
7: to do attrs in alphaequal extensibly
8: handle floating point equality

Tired and Sleepy. M.K. out.

@MarisaKirisame MarisaKirisame force-pushed the roundtrip branch 4 times, most recently from 616ab79 to fedc10f Compare July 12, 2019 18:02
@@ -0,0 +1,317 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
Copy link
Member

Choose a reason for hiding this comment

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

let us keep the base64 and json in the src. The main reason is that we do not want to expose these APIs as public API, but rather internal ones. Likely keeping these files in the original location won't affect anything in the parser. As we only use them via save/load json

@tqchen tqchen merged commit 2973f8a into apache:master Jul 18, 2019
@tqchen
Copy link
Member

tqchen commented Jul 18, 2019

Thanks @MarisaKirisame this PR is now merged

@MarisaKirisame MarisaKirisame deleted the roundtrip branch July 19, 2019 01:48
wweic pushed a commit to wweic/tvm that referenced this pull request Aug 9, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Sep 6, 2019
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.

2 participants