Skip to content

Commit

Permalink
Fix out-of-bounds writes in NumpyOps/CupyOps (#664)
Browse files Browse the repository at this point in the history
* Fix out-of-bounds writes in NumpyOps/CupyOps

- Using `{CupyOps,NumpyOps}.adam` with incompatible shapes for weights,
  gradients, or moments resulted in out-of-bound writes.
- Using `NumpyOps.adam` with non-float32 arrays resulted filling arrays
  with incorrect data.

* Remove print debugging remnants

Co-authored-by: Adriane Boyd <adrianeboyd@gmail.com>

* More print debugging remnants

Co-authored-by: Adriane Boyd <adrianeboyd@gmail.com>

Co-authored-by: Adriane Boyd <adrianeboyd@gmail.com>
  • Loading branch information
danieldk and adrianeboyd committed May 17, 2022
1 parent d2d7917 commit 9063168
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
10 changes: 10 additions & 0 deletions thinc/backends/cupy_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ def scatter_add(self, table, indices, values):
def adam(
self, weights, gradient, mom1, mom2, beta1, beta2, eps, learn_rate, mod_rate=1.0
):
_check_compatible_shape(weights, gradient)
_check_compatible_shape(weights, mom1)
_check_compatible_shape(weights, mom2)

adam_kernel(
gradient, learn_rate, 1 - beta1, 1 - beta2, eps, weights, mom1, mom2
)
Expand All @@ -303,3 +307,9 @@ def position_encode(self, N, D, period=10000, out=None):
)
else:
adam_kernel = None


def _check_compatible_shape(u, v):
if u.shape != v.shape:
msg = f"arrays have incompatible shapes: {u.shape} and {v.shape}"
raise ValueError(msg)
9 changes: 7 additions & 2 deletions thinc/backends/numpy_ops.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,14 @@ class NumpyOps(Ops):

@cython.boundscheck(False)
@cython.wraparound(False)
def adam(self, np.ndarray weights, np.ndarray gradient, np.ndarray mom1,
np.ndarray mom2, const float beta1, const float beta2, float eps,
def adam(self, np.ndarray[np.float32_t] weights, np.ndarray[np.float32_t] gradient,
np.ndarray[np.float32_t] mom1, np.ndarray[np.float32_t] mom2,
const float beta1, const float beta2, float eps,
float learn_rate, float mod_rate=1.):
_check_compatible_shape(weights, gradient)
_check_compatible_shape(weights, mom1)
_check_compatible_shape(weights, mom2)

_adam_momentum(<float*>gradient.data, <float*>mom1.data, <float*>mom2.data,
weights.shape[0], beta1, beta2, eps, learn_rate)
VecVec.add_i(<float*>weights.data,
Expand Down
10 changes: 10 additions & 0 deletions thinc/backends/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,10 @@ def adam(
learn_rate: float,
mod_rate: float = 1.0,
) -> Tuple[Floats1d, Floats1d, Floats1d, Floats1d]:
_check_compatible_shape(weights, gradient)
_check_compatible_shape(weights, mom1)
_check_compatible_shape(weights, mom2)

# Internals for optimizer
mom1 *= beta1
mom2 *= beta2
Expand Down Expand Up @@ -1570,3 +1574,9 @@ def gaussian_cdf(ops: Ops, X: FloatsType) -> FloatsType:
def gaussian_pdf(ops: Ops, X: FloatsType) -> FloatsType:
"""Gaussian PDF for distribution with mean 0 and stdev 1."""
return INV_SQRT_2PI * ops.xp.exp(-0.5 * X * X)


def _check_compatible_shape(u: FloatsXd, v: FloatsXd):
if u.shape != v.shape:
msg = f"arrays have incompatible shapes: {u.shape} and {v.shape}"
raise ValueError(msg)
16 changes: 16 additions & 0 deletions thinc/tests/backends/test_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ def test_ops_consistency(op):
assert str(p1) == str(p2), attr


@pytest.mark.parametrize("ops", ALL_OPS)
def test_adam_incorrect_inputs(ops):
one = ops.xp.zeros(1, dtype="f")
two = ops.xp.zeros(2, dtype="f")

ops.adam(one, one, one, one, 0.0, 0.0, 0.0, 0.0)
with pytest.raises(ValueError):
ops.adam(two, one, one, one, 0.0, 0.0, 0.0, 0.0)
with pytest.raises(ValueError):
ops.adam(one, two, one, one, 0.0, 0.0, 0.0, 0.0)
with pytest.raises(ValueError):
ops.adam(one, one, two, one, 0.0, 0.0, 0.0, 0.0)
with pytest.raises(ValueError):
ops.adam(one, one, one, two, 0.0, 0.0, 0.0, 0.0)


@pytest.mark.parametrize("ops", ALL_OPS)
def test_alloc(ops):
float_methods = (ops.alloc1f, ops.alloc2f, ops.alloc3f, ops.alloc4f)
Expand Down

0 comments on commit 9063168

Please sign in to comment.