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

Revamp tensor constructors. #27

Merged
merged 2 commits into from
Oct 13, 2018

Conversation

dan-zheng
Copy link
Collaborator

Make constructors that take both "shape" and "data" arguments:

  • Specify shape as an explicit Seq[Int], not Int*. This clearly
    distinguishes shape from data.
  • Specify shape as the first argument. This is consistent with libraries
    like NumPy (e.g. np.full(shape, fill_value)).

Rename zeros(Tensor) and ones(Tensor) to zeros_like and ones_like.

Make constructors that take both "shape" and "data" arguments:
- Specify shape as an explicit `Seq[Int]`, not `Int*`. This clearly
  distinguishes shape from data.
- Specify shape as the first argument. This is consistent with libraries
  like NumPy (e.g. `np.full(shape, fill_value)`).

Rename `zeros(Tensor)` and `ones(Tensor)` to `zeros_like` and `ones_like`.
@dan-zheng dan-zheng requested a review from feiwang3311 October 13, 2018 15:33
@feiwang3311 feiwang3311 merged commit 59acc20 into feiwang3311:master Oct 13, 2018
Copy link
Collaborator

@TiarkRompf TiarkRompf left a comment

Choose a reason for hiding this comment

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

Not entirely sure about explicit Seqs. Often multiple parameters lists (i.e., foo(...)(...)) provide enough disambiguation.

@@ -194,7 +194,7 @@ class AdLMSVectorTest extends LanternFunSuite {
@virtualize
def snippet(a: Rep[String]): Rep[Unit] = {
val idx = var_new(0)
val t = Tensor.fill(seq => { idx += 1; idx }, 2, 3)
val t = Tensor.fill(Seq(2, 3), (seq => { idx += 1; idx }))

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer writing this as

val t = Tensor.fill(2, 3) { is => idx += 1; idx }

@dan-zheng
Copy link
Collaborator Author

Multiple parameter lists is a great idea, noted in #17. I'll continue to focus on GPU support.

@TiarkRompf TiarkRompf added this to the M1 milestone Oct 17, 2018
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.

3 participants