A potential bug in dyadic() function?
keitamasuda opened this issue · 3 comments
First of all, this library is fantastic and the code is really clean that gives me a sense of a good library.
Thank you so much for making this available.
But I've found that Matrix by Vector multiplication fails on dyadic()
function.
I still don't know if this is because my usage is wrong or there is a bug.
This error can be reproduced by the following code:
f := func(y *ad.DenseReal64Vector, x *ad.DenseReal64Matrix, w *ad.DenseReal64Vector) ad.MagicScalar {
_, c := x.Dims()
weightedInput := ad.NullDenseReal64Vector(c)
weightedInput.MdotV(x, w)
// Take mean
mean := ad.NullReal64()
mean.Vmean(weightedInput)
return mean
}
xBack := []float64{
1, 1, 1,
1, 1, 1,
1, 1, 1,
}
x := ad.NewDenseReal64Matrix(xBack, 3, 3)
wBack := []float64{
0.5,
0.5,
0.5,
}
w := ad.NewDenseReal64Vector(wBack)
yBack := []float64{
1,
1,
1,
}
y := ad.NewDenseReal64Vector(yBack)
_ = w.Variables(1)
_ = y.Variables(1)
_ = x.Variables(1)
z := f(&y, x, &w)
And here is where the error occurs:
func (c *Real64) dyadic(a, b ConstScalar, v0, v10, v01, v11, v20, v02 float64) *Real64 {
c.AllocForTwo(a, b)
if c.Order >= 1 {
if c.Order >= 2 {
// compute hessian
for i := 0; i < c.GetN(); i++ {
for j := i; j < c.GetN(); j++ {
c.SetHessian(i, j,
a.GetHessian(i, j)*v10 +
b.GetHessian(i, j)*v01 +
a.GetDerivative(i)*a.GetDerivative(j)*v20 +
b.GetDerivative(i)*b.GetDerivative(j)*v02 +
a.GetDerivative(i)*b.GetDerivative(j)*v11 +
b.GetDerivative(i)*a.GetDerivative(j)*v11)
c.SetHessian(j, i, c.GetHessian(i, j))
}
}
}
// compute first derivatives
for i := 0; i < c.GetN(); i++ {
c.SetDerivative(i, a.GetDerivative(i)*v10 + b.GetDerivative(i)*v01) <--- HERE!!!
}
}
// compute new value
c.setFloat64(v0)
return c
}
This is because we have a.GetN() = 9
where as b.GetN() = 3
resulting in the following error:
panic: runtime error: index out of range [3] with length 3 [recovered]
panic: runtime error: index out of range [3] with length 3
The code should be like this? I'm not expert on autodiff so I'm not sure but this runs without error.
// compute first derivatives
for i := 0; i < a.GetN(); i++ {
for j :=0; j < b.GetN(); j++ {
c.SetDerivative(i, a.GetDerivative(i)*v10 + b.GetDerivative(j)*v01)
}
}
If this is okay, can I send a pull request?
Thank you for your interest in autodiff! The method Variables() should only be called once. So either w.Variables(1), y.Variables(1), or x.Variables(1). If you need the partial derivatives for all variables you must allocate a single vector for w, y, x, call Variables() and then split the vector.
But I see why this is confusing and it is not documented well. I will implement a better error message and update the documentation. Thank you!
Done
@pbenner
Thank you for your comment and your implementation. That was fast!
I'll still be using this library to do my research. I hope that someday I can contribute to this.