`Manifest` -> `Type` change proposal by Angelogeb · Pull Request #75 · TiarkRompf/lms-clean

Angelogeb · 2021-02-01T15:45:51Z

This PR introduces a Type typeclass based representation of types in LMS.

This implementation is based on the one present on the previous version of LMS and the one presented in
"Reflections on LMS: exploring front-end alternatives".

Rationale

LMS relies on Manifests in Adapter.typeMap for keeping track of types of nodes in the graph. Although manifests are in general only used in the backend for code generation, there are cases where we would like to provide additional information in the types of the nodes.
This has so far been overcome in ad-hoc ways adding this information at the Node level outside of the typeMap (example1, example2). Although convenient, this is not uniform and requires additional code to keep the information in sync between the typeMap and the nodes.

Design

The typeMap is updated from a Map[Exp, Manifest[_]] to Map[Exp, TypeExp].
Types are represented as graph terms. This might allow using LMS in new contexts.
Types are reflected on the graph by Wrap
To introduce new types a Type instance must be provided
The Type type class provides two reify methods: one for simple types and one for types
that might need to inspect the Expression before reflecting a type (in the case of the tensors example2 above,
this allows to promote the shape to the type level). I am not sure about the latter design decision and if there are
alternatives.

Open questions

Should the type nodes be in the same graph as the expressions?
- Do we have to change some parts of the backend to ignore such nodes or are they ignored by default?

Tasks

Typeclass sketch with example in Main to be deleted
Extend interface for easier migration (equivalent of manifest[] etc.)
Adapter.typeMap and Adapter.oldTypeMap: HashMap[Exp, Manifest[_]] -> HashMap[Exp, TypeExp]
Wrap[T:Manifest] -> Wrap[T:Type]
Update code using Manifest to Type
- This will be a bit time consuming because the usage of Manifests hasn't been abstracted away using UtilOps which would have allowed using refactoring tools.

src/main/scala/lms/core/types.scala

feiwang3311 · 2021-02-01T17:52:56Z

src/main/scala/lms/core/types.scala

+  val Unit:     TypeExp = Const(TypeNames.Unit)
+  val Boolean:  TypeExp = Const(TypeNames.Boolean)
+  val Char:     TypeExp = Const(TypeNames.Char)
+  val Short:    TypeExp = Const(TypeNames.Short)
+  val Int:      TypeExp = Const(TypeNames.Int)
+  val Float:    TypeExp = Const(TypeNames.Float)
+  val Double:   TypeExp = Const(TypeNames.Double)


would these names conflict with the types of Scala?

Having the same name helps in identifying what corresponds to what but I am open to changes. See cd0a859 on a possible way to ensure that they don't conflict but I am open to different names

feiwang3311 · 2021-02-01T17:57:57Z

src/main/scala/lms/core/types.scala

+}
+
+object ArrayType {
+  val name = "Array"


this is building type with type parameters. I am slightly concerned if the name of the node op should follow some specific rules, such that we don't accidentally use the same node op name for terms and types.
For instance, maybe for types, the name is always T-Array with a T-?
This is probably a bad suggestion. Hopefully you guys have better ideas. :)

Indeed I was also thinking to prefix such names. T- sounds reasonable

src/main/scala/lms/core/types.scala

TiarkRompf · 2021-02-08T03:16:24Z

Thanks for having a go at this! This looks like a good start. Couple questions/remarks:

My mental model so far has been that typeMap: HashMap[Exp,Exp] needs to be part of a central class such as GraphBuilder itself, so I'm curious about the trade-offs with the approach taken here.
As a general scheme I'd propose to name things ArrayT, TensorT, FunT, etc when referring to the term that represents the types, so a dependent function type constructor could be FunT(argtpe => ... restpe)
I think we need to "play through" a couple more test cases to see how things work. In particular I'm thinking of a minimal extension of the simply-typed API in frontend.scala, perhaps keeping the same user-facing API but representing and checking/propagating types internally. This should include all relevant test cases.
What will lambda calculus with dependent types look like? See here on how to implement.
For tensor types, what will be the type of matmul, written using fun?

Happy to meet and discuss, or for you to do another iteration first, whichever you prefer.

TiarkRompf · 2021-02-08T02:50:36Z

src/main/scala/lms/core/types.scala

+  }
+
+}
+


Interesting model -- I was thinking more along the lines of putting the HashMap into the GraphBuilder class. What are pros and cons?

The current typeMap is part of the global state in Adapter. The new version of TypeMap probably won't have a GraphBuilder and will use the one that is used for reflecting terms Adapter.g.

A GraphBuilder object is not long-living. After a Graph is constructed, the GraphBuilder object is used to generate a Graph object, and then discarded. If we have a HashMap in the GraphBuilder, we might also need to save it to the Graph object. It might not be a bad idea.

So far we have been using Metadata to track additional information for Graph. When doing transformation, the handling of Metadata gets hacky (using Adapter.oldTypeMap, for instance). If this Metadata is part of the Graph, then that looks cleaner imo.

src/main/scala/lms/core/types.scala

TiarkRompf · 2021-02-08T03:21:30Z

src/main/scala/lms/core/types.scala

+object Reflect {
+  def unapply(x: Any)(implicit tm: TypeMap): Option[(String, List[Exp])] = x match {
+    case s@Sym(_) =>
+      tm.g.findDefinition(s).map(n => (n.op, n.rhs.filter(_.isInstanceOf[Exp]).map(_.asInstanceOf[Exp])))
+    case _ => None
+  }
+}


Why is this extractor needed? Can't we use Def from GraphBuilder?

Angelogeb · 2021-02-09T14:26:09Z

I think we need to "play through" a couple more test cases to see how things work. In particular I'm thinking of a minimal extension of the simply-typed API in frontend.scala, perhaps keeping the same user-facing API but representing and checking/propagating types internally. This should include all relevant test cases.

What will lambda calculus with dependent types look like? See here on how to implement.

For tensor types, what will be the type of matmul, written using fun?

Happy to meet and discuss, or for you to do another iteration first, whichever you prefer.

Currently working on this

feiwang3311 · 2021-02-15T19:25:07Z

src/main/scala/lms/core/types.scala

+}
+
+object TypeNames extends Enumeration {
+  val InferredT, UnitT, BooleanT, CharT, ShortT, IntT, FloatT, DoubleT = Value


what is InferredT?

Angelogeb added 3 commits February 1, 2021 09:56

Initial Type typeclass sketch

6f41317

Reformat

010035b

Refactor

f982810

feiwang3311 reviewed Feb 1, 2021

View reviewed changes

src/main/scala/lms/core/types.scala Outdated Show resolved Hide resolved

feiwang3311 reviewed Feb 1, 2021

View reviewed changes

src/main/scala/lms/core/types.scala Outdated Show resolved Hide resolved

Fixes

cd0a859

TiarkRompf reviewed Feb 8, 2021

View reviewed changes

Major rewrite, added ExperimentalBase and matMul example

88ca928

feiwang3311 reviewed Feb 15, 2021

View reviewed changes

Conversation

Angelogeb commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Design

Open questions

Tasks

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TiarkRompf commented Feb 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Angelogeb commented Feb 9, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Angelogeb commented Feb 1, 2021 •

edited

Loading