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

Additional performance related stuff #391

Merged
merged 15 commits into from Jun 1, 2020
Merged

Additional performance related stuff #391

merged 15 commits into from Jun 1, 2020

Conversation

chewxy
Copy link
Member

@chewxy chewxy commented Apr 10, 2020

No description provided.

@chewxy chewxy mentioned this pull request May 17, 2020
Originally due to the behaviour of the `sliceOp` at runtime, the resulting tensor is not materialized.
This made things very difficult for optimization - it's cheaper to materialize each step of a slice.
As a result, the raw .Data() no longer contains the extra data.

A concrete example:

```
a := NewMatrix(g, float64, WithShape(3, 2), WithName("a"), WithInit(RangedFrom(0)))
x := Slice(a, nil,  S(0))
m := NewTapeMachine(g)

m.RunAll()
x.Value()
// will be a colvec:
// [0]
// [2]
// [4]

```

In the old version, the slice operation doesn't materialize, so `x.Value()` is the same, but the raw data, acquired by `.Data()` is `[0, 1, 2, 3, 4]`

Now it has been materialized, so the raw data from `x.Value().Data()` will simply be `[0 2 4]`
@coveralls
Copy link

coveralls commented May 18, 2020

Coverage Status

Coverage increased (+0.1%) to 64.274% when pulling fc9d1b7 on v0.9.10-performance into 5fb5944 on master.

MarkKremer added a commit to MarkKremer/gorgonia that referenced this pull request Jun 1, 2020
@chewxy chewxy merged commit 3eb51b4 into master Jun 1, 2020
chewxy added a commit that referenced this pull request Jun 3, 2020
* Copy input shape for batchedMatMulOp instead of append

* Undo fix to make place for #391, keep tests

Co-authored-by: Chewxy <chewxy@gmail.com>
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.

None yet

2 participants