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

Fix reduction bugs #357

Merged
merged 2 commits into from Dec 30, 2019
Merged

Fix reduction bugs #357

merged 2 commits into from Dec 30, 2019

Conversation

@chewxy
Copy link
Member

chewxy commented Dec 30, 2019

Fixes reduction bugs while cleaning up some of the code.

  • len(shape) is to be discouraged, as future versions of tensor.Shape may be more static for performance reasons.
  • reductionInerSahpe does not require ...DimSizer as inputs. Instead, now checks are made so that they only take tensor.Shape.

The bug being resolved can be described as follows:

func TestSumOpFakeVec(t *testing.T) {
	g := NewGraph()

	xv := tensor.New(tensor.WithBacking([]float64{1, 2}), tensor.WithShape(2, 1))
	yv := tensor.New(tensor.WithBacking([]float64{10, 20}), tensor.WithShape(1, 2))
	x := NewMatrix(g, Float64, WithName("x"), WithShape(2, 1), WithValue(xv))
	y := NewMatrix(g, Float64, WithName("y"), WithShape(1, 2), WithValue(yv))
	sx, _ := Sum(x)
	sy, _ := Sum(y)

	assert.True(t, sx.Shape().Eq(tensor.ScalarShape()))
	assert.True(t, sy.Shape().Eq(tensor.ScalarShape()))

	sx2, _ := Sum(x, 1)
	assert.True(t, sx2.Shape().Eq(tensor.Shape{2}))

	vm := NewTapeMachine(g)
	vm.RunAll()

	assert.Equal(t, 3.0, sx.Value().Data(), "Expected sx to be 3.0")
	assert.Equal(t, 30.0, sy.Value().Data(), "Expected sy to be 30.0")
	assert.Equal(t, []float64{1, 2}, sx2.Value().Data(), "sx2 should be a flat array")
}

This test has also been added

The cause of the bug is that reductionInferShape and reductionType do not agree with each other.

This is due to an incompletely defined type system which hopefully will be fixed in the upcoming version.

The current solution is a hack: we treat colvec and rowvec matrices as vectors in type inference.

chewxy added 2 commits Dec 29, 2019
@coveralls

This comment has been minimized.

Copy link

coveralls commented Dec 30, 2019

Coverage Status

Coverage increased (+0.01%) to 63.384% when pulling 3d134f9 on fixReductionBugs into a8bd935 on master.

@chewxy chewxy merged commit 1e98951 into master Dec 30, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@chewxy chewxy deleted the fixReductionBugs branch Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.