You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tvm.apache.org by Josh Pollock <no...@github.com> on 2019/04/12 22:12:05 UTC

[dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

The following RFC is for more bikeshedding about syntax of the Relay text format not covered by #1782.

The guiding principles for syntax choices are
- **readability:** Syntax should be easy to read and parse visually.
- **internal consistency:** Syntax choices should be similar to each other (e.g. function definitions and function types should have similar syntax).
- **external consistency:** We should strive to reuse existing syntax choices from other languages so users that are familiar with those languages have less to learn. Also existing syntax choices are _usually_ well-thought-out.

**Please provide feedback on the syntax choices below or other syntactic features you think should be discussed that weren't already agreed on in #1782.**

## References
### RefCreate
Existing languages:
OCaml
```
ref 5
```
ReasonML
```
ref(5)
```
Relay Proposal
```
ref(5)
```
### RefRead
OCaml
```
r := v
```
ReasonML
```
r := v
```
Relay Proposal
```
r := v
```
### RefWrite
OCaml
```
!r
```
ReasonML (adopted because `!` is already used for `not`)
```
r^
```
It may be temping to use `&x`, but this implies that any expression has a reference, whereas in ML-like languages references must be created explicitly.
Relay Proposal
```
*r
```
or
```
read(r)
```
Prefixes seem better than postfixes, but we need to avoid `!` do to ambiguity. I'm open to other suggestions.

## ADTs
ReasonML
```
type myVariant =
  | HasNothing
  | HasSingleInt(int)
  | HasSingleTuple((int, int))
  | HasMultipleInts(int, int)
  | HasMultipleTuples((int, int), (int, int))
```
Rust
```
enum myVariant {
  HasNothing,
  HasSingleInt(int),
  HasSingleTuple((int, int)),
  HasMultipleInts(int, int),
  HasMultipleTuples((int, int), (int, int)),
}
```
Relay Proposal
```
type myVariant =
  | HasNothing
  | HasSingleInt(int)
  | HasSingleTuple((int, int))
  | HasMultipleInts(int, int)
  | HasMultipleTuples((int, int), (int, int))
```
## Pattern Matching
ReasonML
```
switch (x) {
  | HasNothing => 0
  | HasSingleInt(x) => 0
  | HasSingleTuple((x, y)) => 0
  | HasMultipleInts(x, y) => 0
  | HasMultipleTuples((x, y), (q, r)) => 0
}
```
Rust
```
match x {
  HasNothing => 0,
  HasSingleInt(x) => 0,
  HasSingleTuple((x, y)) => 0,
  HasMultipleInts(x, y) => 0,
  HasMultipleTuples((x, y), (q, r)) => 0,
}
```
Relay Proposal
```
match (x) {
  | HasNothing => 0
  | HasSingleInt(x) => 0
  | HasSingleTuple((x, y)) => 0
  | HasMultipleInts(x, y) => 0
  | HasMultipleTuples((x, y), (q, r)) => 0
}
```
`case` is also a viable alternative keyword.
## Generics (i.e. type arguments)
Rust
```
enum Option<T> {
    Some(T),
    None,
}

fn takes_anything<T>(x: T) {
    // do something with x
}
```
```
let x: Option<i32> = Some(5);
let x: Option<i64> = Some(5);
```
ReasonML
```
type Option('a) =
    | Some('a)
    | None
```
Scala
```
Some[A](a : A)
```
Python
```
Option[A] = Union[A, None]
```
Relay Proposal
```
type Option[A] =
  | Some(A)
  | None

fn takes_anything[T](%x: T) {
    // do something with %x
}

let x = Some[i32](5);
let x = Some[i64](5);
```

## TupleGetItem
ReasonML
```
("foo",2,true).0 // "foo"
```
Rust
```
("foo",2,true).0 // "foo"
```
Scala
```
("foo",2,true)._1 // "foo"
```
Python
```
("foo",2,true)[0] # "foo"
```
Relay Proposal
```
("foo",2,true).0 // "foo"
```

## Semicolons
We need to consider whether or not to use semicolons as separators in sequence operations. The pros of using semicolons is they're easier to implement and allow us to ignore newline characters when writing and parsing programs. The cons are they are more cumbersome and make the text format less python-like.

## Attributes
Python
```
def foo(a, b, c=None, d=None):
```
Relay Proposal
```
@foo(%a, %b, %c=None, %d=None)
```
when the attrs type index matches the operator's attrs type index
```
@foo(%a, %b, BarAttrs={%c: None, %d: None})
```
when it doesn't.
I'm not as certain about this syntax piece, so I'm very open to suggestions here.

cc @tqchen @MarisaKirisame @jroesch @wweic @vinx13 @junrushao1994 @nhynes

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Wei Chen <no...@github.com>.
@MarisaKirisame Thanks for clarification. Just curious about the implementation details. I don't have a use case right now. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-482890476

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Josh Pollock <no...@github.com>.
@tqchen could you provide examples of the `attrs=ValueFormat` suggestion?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-484319295

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Josh Pollock <no...@github.com>.
@wweic thanks I fixed the typo

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-483056033

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Tianqi Chen <no...@github.com>.
This RFC isa bit out-dated, I would recommend close it, and open new checklist for the next todos

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-539744523

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Josh Pollock <no...@github.com>.
This PR has been open for a while now. Please post your final thoughts so we can close this and begin implementing the language features. In particular, we should try to come to some resolution on attributes.

@tqchen @MarisaKirisame @jroesch @wweic @vinx13 @junrushao1994 @nhynes

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-490294434

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by 雾雨魔理沙 <no...@github.com>.
My single objection is no ^.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-490294945

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Wei Chen <no...@github.com>.
`RefRead` and `RefRead` are swapped in the spec. 

For `@foo(%a, %b, BarAttrs={%c: None, %d: None})`, does it mean Relay supports record? Or Relay core recognizes the record like syntax and maintains the record internally, source code can not access the record?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-482818422

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Tianqi Chen <no...@github.com>.
Please conclude and summarize the RFC so we could start a vote about the text format

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-501900987

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Wei Chen <no...@github.com>.
I'm good with the RFC overall. Slightly prefer `!` same as @MarisaKirisame.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-490325592

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Tianqi Chen <no...@github.com>.
- Semi-colons: we should not have them, to make most syntax consistent with python
- attrs when it does not match attr's type index
  - This needs more deliberation, it needs to be consistent with the case when we don't print out BarAttrs(in the meta-data). Or print BarAttrs directly(although not allowed atm), My recommendation simply use attrs=ValueFormat, where ValueFormat can be any printed format of the attrs

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-482869701

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Wei Chen <no...@github.com>.
We might also need to support `Any` https://github.com/dmlc/tvm/issues/3042.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-532348178

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Tianqi Chen <no...@github.com>.
Closed #3016.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#event-2697166502

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by Josh Pollock <no...@github.com>.
Here is a summary of the proposed text format changes.

**RefCreate**: `ref(n)`
**RefWrite**: `r := v`
**RefRead**: `!r`
**ADTs**:
```
type myVariant =
  | HasNothing
  | HasSingleInt(int)
  | HasSingleTuple((int, int))
  | HasMultipleInts(int, int)
  | HasMultipleTuples((int, int), (int, int))
```
**Pattern Matching**:
```
match (x) {
  | HasNothing => 0
  | HasSingleInt(x) => 0
  | HasSingleTuple((x, y)) => 0
  | HasMultipleInts(x, y) => 0
  | HasMultipleTuples((x, y), (q, r)) => 0
}
```
**Generics**
```
type Option[A] =
  | Some(A)
  | None

fn takes_anything[T](%x: T) {
    // do something with %x
}

let x = Some[i32](5);
let x = Some[i64](5);
```
**TupleGetItem**: `("foo", 2, true).0 // "foo"`
**Semicolons**: optional unless collapsing multiple lines into one as in Python
**Attrs**: We still don't have a satisfying answer for this so we may punt it. We will default to Tianqi's suggestion of `attrs=ValueFormat` where we allow different options for `ValueFormat`.

**Please vote on these changes.** This will allow us to nearly solidify the text format.
cc @tqchen @MarisaKirisame @jroesch @wweic @vinx13 @junrushao1994 @nhynes


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-504065532

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by 雾雨魔理沙 <no...@github.com>.
@joshpoll what about type inference? How will it work if the type parameter is inferred? e.g. Some(5)?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-504163322

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by 雾雨魔理沙 <no...@github.com>.
The Attribute System achieved a record syntax.
We see the point of record though, we just do not have enough man power and it is not high priority. If you need them, the easiest way is to probably use ADT instead (fields can be function implemented by pattern matching). Or if you really need them, can you tell me the use case?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-482868316

Re: [dmlc/tvm] [RFC][Relay] Text Format Part 2 (#3016)

Posted by 雾雨魔理沙 <no...@github.com>.
No No No No No on r^.
I'll still say !r is the best - not doesnt appear much in deep learning, and should not take !.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/dmlc/tvm/issues/3016#issuecomment-482753437