ronantakizawa/codereview-bench huggingface dataset:
Repository: numpy/numpy
File: numpy/linalg/_linalg.py (Python)

Code before:
           page 795.

    Examples
    --------
    >>> import numpy as np
    >>> from numpy.linalg import matrix_rank
    >>> matrix_rank(np.eye(4)) # Full rank matrix
    4
    >>> I=np.eye(4); I[-1,-1] = 0. # rank deficient matrix
    >>> matrix_rank(I)
    3
    >>> matrix_rank(np.ones((4,))) # 1 dimension - rank 1 unless all 0
    1
    >>> matrix_rank(np.zeros((4,)))
    0
    """
    if rtol is not None and tol is not None:
        raise ValueError("`tol` and `rtol` can't be both set.")

    A = asarray(A)
    if A.ndim < 2:
        return int(not all(A == 0))

    # Handle empty matrices - rank is 0 for matrices with 0 rows or 0 columns
    if A.size == 0:
        return 0

    S = svd(A, compute_uv=False, hermitian=hermitian)

    if tol is None:
        if rtol is None:
            rtol = max(A.shape[-2:]) * finfo(S.dtype).eps
        else:
            rtol = asarray(rtol)[..., newaxis]
        tol = S.max(axis=-1, keepdims=True) * rtol
    else:
        tol = asarray(tol)[..., newaxis]

    return count_nonzero(S > tol, axis=-1)


# Generalized inverse

def _pinv_dispatcher(a, rcond=None, hermitian=None, *, rtol=None):
    return (a,)


@array_function_dispatch(_pinv_dispatcher)
def pinv(a, rcond=None, hermitian=False, *, rtol=_NoValue):
    """
    Compute the (Moore-Penrose) pseudo-inverse of a matrix.

Review comment (bug):
I wonder if we should just add `initial=0` which I assume also works. The tests you added are missing the case of `np.zeros((0, 5, 5))` where this return is clearly incorrect.

EDIT: Actually, while we are here, can you check if we don't return `np.intp(0)` on the last return?  We may want to adjust the return type for the other early return as well.

Code after:
           page 795.

    Examples
    --------
    >>> import numpy as np
    >>> from numpy.linalg import matrix_rank
    >>> matrix_rank(np.eye(4)) # Full rank matrix
    4
    >>> I=np.eye(4); I[-1,-1] = 0. # rank deficient matrix
    >>> matrix_rank(I)
    3
    >>> matrix_rank(np.ones((4,))) # 1 dimension - rank 1 unless all 0
    1
    >>> matrix_rank(np.zeros((4,)))
    0
    """
    if rtol is not None and tol is not None:
        raise ValueError("`tol` and `rtol` can't be both set.")

    A = asarray(A)
    if A.ndim < 2:
        return int(not all(A == 0))

    S = svd(A, compute_uv=False, hermitian=hermitian)

    if tol is None:
        if rtol is None:
            rtol = max(A.shape[-2:]) * finfo(S.dtype).eps
        else:
            rtol = asarray(rtol)[..., newaxis]
        tol = S.max(axis=-1, keepdims=True, initial=0) * rtol
    else:
        tol = asarray(tol)[..., newaxis]

    return count_nonzero(S > tol, axis=-1)


# Generalized inverse

def _pinv_dispatcher(a, rcond=None, hermitian=None, *, rtol=None):
    return (a,)


@array_function_dispatch(_pinv_dispatcher)
def pinv(a, rcond=None, hermitian=False, *, rtol=_NoValue):
    """
    Compute the (Moore-Penrose) pseudo-inverse of a matrix.

    Calculate the generalized inverse of a matrix using its
    singular-value decomposition (SVD) and including all
    *large* singular values.
Repository: numpy/numpy
File: numpy/_core/tests/test_multithreading.py (Python)

Code before:
                    assert expected_warning in str(ex)

    run_threaded(func, max_workers=10, pass_count=True, outer_iterations=5)


# These are all implemented using PySequence_Fast, which needs locking to be safe
def np_broadcast(arrs):
    for i in range(100):
        np.broadcast(arrs)

def create_array(arrs):
    for i in range(100):
        np.array(arrs)

def create_nditer(arrs):
    for i in range(1000):
        np.nditer(arrs)


@pytest.mark.parametrize(
    "kernel, outcome",
    (
        (np_broadcast, "error"),
        (create_array, "error"),
        (create_nditer, "success")
    )
)
def test_arg_locking(kernel, outcome):
    # should complete without triggering races but may error
    # changing

    b = threading.Barrier(5)
    done = 0
    arrs = []

    def read_arrs():
        nonlocal done
        b.wait()
        try:
            kernel(arrs)
        finally:
            done += 1

    def mutate_list():
        b.wait()
        while done < 4:
            if len(arrs) > 10:
                arrs.pop(0)
            elif len(arrs) <= 10:
                arrs.extend([np.array([1, 2, 3]) for _ in range(1000)])


Review comment (suggestion):
micro-nit

```suggestion
        (create_nditer, "success"),
    ),
```

Code after:
                    assert expected_warning in str(ex)

    run_threaded(func, max_workers=10, pass_count=True, outer_iterations=5)


# These are all implemented using PySequence_Fast, which needs locking to be safe
def np_broadcast(arrs):
    for i in range(50):
        np.broadcast(arrs)

def create_array(arrs):
    for i in range(50):
        np.array(arrs)

def create_nditer(arrs):
    for i in range(50):
        np.nditer(arrs)


@pytest.mark.parametrize(
    "kernel, outcome",
    (
        (np_broadcast, "error"),
        (create_array, "error"),
        (create_nditer, "success"),
    ),
)
def test_arg_locking(kernel, outcome):
    # should complete without triggering races but may error

    done = 0
    arrs = [np.array([1, 2, 3]) for _ in range(1000)]

    def read_arrs(b):
        nonlocal done
        b.wait()
        try:
            kernel(arrs)
        finally:
            done += 1

    def contract_and_expand_list(b):
        b.wait()
        while done < 4:
            if len(arrs) > 10:
                arrs.pop(0)
            elif len(arrs) <= 10:
                arrs.extend([np.array([1, 2, 3]) for _ in range(1000)])

    def replace_list_items(b):
        b.wait()
Repository: numpy/numpy
File: benchmarks/benchmarks/bench_searchsorted.py (Python)

Code before:
import numpy as np

from .common import Benchmark


class SearchSortedInt64(Benchmark):
    # Benchmark for np.searchsorted with int64 arrays
    params = [
        # 1B u64 is 8gb
        [100, 10_000, 1_000_000, 1_000_000_000],       # array sizes
        [1, 2, 100, 100_000],   # number of query elements
        ['ordered', 'random'],        # query order
        [42, 18122022]
    ]
    param_names = ['array_size', 'n_queries', 'query_order', 'seed']

    def setup(self, array_size, n_queries, query_order, seed):
        self.arr = np.arange(array_size, dtype=np.int64)

        rng = np.random.default_rng(seed)

        low = -array_size // 10
        high = array_size + array_size // 10
        self.queries = rng.integers(low, high, size=n_queries, dtype=np.int64)

        # Generate queries
        if query_order == 'ordered':
            self.queries.sort()

    def time_searchsorted(self, array_size, n_queries, query_order, seed):
        np.searchsorted(self.arr, self.queries)


Review comment (suggestion):
```suggestion
        [42, 18122022],   # seed
```

Code after:
import numpy as np

from .common import Benchmark


class SearchSorted(Benchmark):
    params = [
        [100, 10_000, 1_000_000, 100_000_000],  # array sizes
        [1, 10, 100_000],                       # number of query elements
        ['ordered', 'random'],                  # query order
        [False, True],                          # use sorter
        [42, 18122022],                         # seed
    ]
    param_names = ['array_size', 'n_queries', 'query_order', 'use_sorter', 'seed']

    def setup(self, array_size, n_queries, query_order, use_sorter, seed):
        self.arr = np.arange(array_size, dtype=np.int32)

        rng = np.random.default_rng(seed)

        low = -array_size // 10
        high = array_size + array_size // 10

        self.queries = rng.integers(low, high, size=n_queries, dtype=np.int32)
        if query_order == 'ordered':
            self.queries.sort()

        if use_sorter:
            rng.shuffle(self.arr)
            self.sorter = self.arr.argsort()
        else:
            self.sorter = None

    def time_searchsorted(self, array_size, n_queries, query_order, use_sorter, seed):
        np.searchsorted(self.arr, self.queries, sorter=self.sorter)
Repository: numpy/numpy
File: benchmarks/benchmarks/bench_searchsorted.py (Python)

Code before:
import numpy as np

from .common import Benchmark


class SearchSorted(Benchmark):
    params = [
        [100, 10_000, 1_000_000, 100_000_000],  # array sizes
        [1, 10, 100_000],                       # number of query elements
        ['ordered', 'random'],                  # query order
        [False, True],                          # use sorted
        [42, 18122022],                         # seed
    ]
    param_names = ['array_size', 'n_queries', 'query_order', 'use_sorter', 'seed']

    def setup(self, array_size, n_queries, query_order, use_sorter, seed):
        self.arr = np.arange(array_size, dtype=np.int32)

        rng = np.random.default_rng(seed)

        low = -array_size // 10
        high = array_size + array_size // 10

        self.queries = rng.integers(low, high, size=n_queries, dtype=np.int32)
        if query_order == 'ordered':
            self.queries.sort()

        if use_sorter:
            rng.shuffle(self.arr)
            self.sorter = self.arr.argsort()
        else:
            self.sorter = None

    def time_searchsorted(self, array_size, n_queries, query_order, use_sorter, seed):
        np.searchsorted(self.arr, self.queries, sorter=self.sorter)


Review comment (suggestion):
```suggestion
        [False, True],                          # use sorter
```

Code after:
import numpy as np

from .common import Benchmark


class SearchSorted(Benchmark):
    params = [
        [100, 10_000, 1_000_000, 100_000_000],  # array sizes
        [1, 10, 100_000],                       # number of query elements
        ['ordered', 'random'],                  # query order
        [False, True],                          # use sorter
        [42, 18122022],                         # seed
    ]
    param_names = ['array_size', 'n_queries', 'query_order', 'use_sorter', 'seed']

    def setup(self, array_size, n_queries, query_order, use_sorter, seed):
        self.arr = np.arange(array_size, dtype=np.int32)

        rng = np.random.default_rng(seed)

        low = -array_size // 10
        high = array_size + array_size // 10

        self.queries = rng.integers(low, high, size=n_queries, dtype=np.int32)
        if query_order == 'ordered':
            self.queries.sort()

        if use_sorter:
            rng.shuffle(self.arr)
            self.sorter = self.arr.argsort()
        else:
            self.sorter = None

    def time_searchsorted(self, array_size, n_queries, query_order, use_sorter, seed):
        np.searchsorted(self.arr, self.queries, sorter=self.sorter)
Repository: numpy/numpy
File: numpy/_core/src/multiarray/descriptor.c (C)

Code before:
        return (PyObject *)arraydescr_field_subset_view(lself, op);
    }
    else {
        Py_ssize_t i = PyArray_PyIntAsIntp(op);
        if (error_converting(i)) {
            /* if converting to an int gives a type error, adjust the message */
            PyObject *err = PyErr_Occurred();
            if (PyErr_GivenExceptionMatches(err, PyExc_TypeError)) {
                PyErr_SetString(PyExc_TypeError,
                        "Field key must be an integer field offset, "
                        "single field name, or list of field names.");
            }
            return NULL;
        }
        return _subscript_by_index(lself, i);
    }
}

static PyObject *
array_typestr_get(PyArray_Descr *self)
{
    return arraydescr_protocol_typestr_get(self, NULL);
}


PyObject *
array_protocol_descr_get(PyArray_Descr *self)
{
    PyObject *res;
    PyObject *dobj;

    res = arraydescr_protocol_descr_get(self, NULL);
    if (res) {
        return res;
    }
    PyErr_Clear();

    /* get default */
    dobj = PyTuple_New(2);
    if (dobj == NULL) {
        return NULL;
    }
    PyTuple_SET_ITEM(dobj, 0, PyUnicode_FromString(""));
    PyTuple_SET_ITEM(dobj, 1, array_typestr_get(self));
    res = PyList_New(1);
    if (res == NULL) {
        Py_DECREF(dobj);
        return NULL;
    }
    PyList_SET_ITEM(res, 0, dobj);
    return res;

Review comment (suggestion):
```suggestion
NPY_NO_EXPORT PyObject *
```
(probably doesn't matter since it's in the header, but...

Code after:
        return (PyObject *)arraydescr_field_subset_view(lself, op);
    }
    else {
        Py_ssize_t i = PyArray_PyIntAsIntp(op);
        if (error_converting(i)) {
            /* if converting to an int gives a type error, adjust the message */
            PyObject *err = PyErr_Occurred();
            if (PyErr_GivenExceptionMatches(err, PyExc_TypeError)) {
                PyErr_SetString(PyExc_TypeError,
                        "Field key must be an integer field offset, "
                        "single field name, or list of field names.");
            }
            return NULL;
        }
        return _subscript_by_index(lself, i);
    }
}

static PyObject *
array_typestr_get(PyArray_Descr *self)
{
    return arraydescr_protocol_typestr_get(self, NULL);
}


NPY_NO_EXPORT PyObject *
array_protocol_descr_get(PyArray_Descr *self)
{
    PyObject *res;
    PyObject *dobj;

    res = arraydescr_protocol_descr_get(self, NULL);
    if (res) {
        return res;
    }
    PyErr_Clear();

    /* get default */
    dobj = PyTuple_New(2);
    if (dobj == NULL) {
        return NULL;
    }
    PyTuple_SET_ITEM(dobj, 0, PyUnicode_FromString(""));
    PyTuple_SET_ITEM(dobj, 1, array_typestr_get(self));
    res = PyList_New(1);
    if (res == NULL) {
        Py_DECREF(dobj);
        return NULL;
    }
    PyList_SET_ITEM(res, 0, dobj);
    return res;
Repository: numpy/numpy
File: numpy/_core/src/multiarray/multiarraymodule.h (C/C++)

Code before:
#ifndef NUMPY_CORE_SRC_MULTIARRAY_MULTIARRAYMODULE_H_
#define NUMPY_CORE_SRC_MULTIARRAY_MULTIARRAYMODULE_H_

#ifdef __cplusplus
extern "C" {
#endif

/*
 * A struct storing global state for the _multiarray_umath
 * module. The state is initialized when the module is imported
 * so no locking is necessary to access it.
 */
typedef struct npy_global_state_struct {
    /*
     * Used to test the internal-only scaled float test dtype
     */
    npy_bool get_sfloat_dtype_initialized;

    /*
     * controls the global madvise hugepage setting
     */
    int madvise_hugepage;

    /*
     * used to detect module reloading in the reload guard
     */
    int reload_guard_initialized;

    /*
     * Holds the user-defined setting for whether or not to warn
     * if there is no memory policy set
     */
    int warn_if_no_mem_policy;
} npy_global_state_struct;



Review comment (suggestion):
```suggestion
 * so no locking is necessary to access it.
 *
 * These globals will need to move to per-module state to
 * support reloading or subinterpreters.
```

Maybe worth adding this note for future readers who want to add support for subinterpreters - which should be much easier now that we've done a nice job of isolating all this state!

Code after:
#ifndef NUMPY_CORE_SRC_MULTIARRAY_MULTIARRAYMODULE_H_
#define NUMPY_CORE_SRC_MULTIARRAY_MULTIARRAYMODULE_H_

#ifdef __cplusplus
extern "C" {
#endif

/*
 * A struct storing global state for the _multiarray_umath
 * module. The state is initialized when the module is imported
 * so no locking is necessary to access it.
 *
 * These globals will need to move to per-module state to
 * support reloading or subinterpreters.
 */
typedef struct npy_global_state_struct {
    /*
     * Used to test the internal-only scaled float test dtype
     */
    npy_bool get_sfloat_dtype_initialized;

    /*
     * controls the global madvise hugepage setting
     */
    int madvise_hugepage;

    /*
     * used to detect module reloading in the reload guard
     */
    int reload_guard_initialized;

    /*
     * Holds the user-defined setting for whether or not to warn
     * if there is no memory policy set
     */
    int warn_if_no_mem_policy;
Repository: numpy/numpy
File: numpy/_core/src/common/npy_hashtable.c (C)

Code before:
}


static int
_resize_if_necessary(PyArrayIdentityHash *tb)
{
#ifdef Py_GIL_DISABLED
    assert(PyMutex_IsLocked(&tb->mutex));
#endif
    struct buckets *old_buckets = tb->buckets;
    int key_len = tb->key_len;
    npy_intp new_size = 0, prev_size = old_buckets->size;
    assert(prev_size > 0);

    if ((old_buckets->nelem + 1) * 2 > prev_size) {
        /* Double in size */
        new_size = prev_size * 2;
    }

    if (new_size == 0) {
        /* No resize necessary */
        return 0;
    }

    npy_intp alloc_size;
    if (npy_mul_sizes_with_overflow(&alloc_size, new_size, key_len + 1)) {
        return -1;
    }
    struct buckets *new_buckets = (struct buckets *)PyMem_Calloc(
        1, sizeof(struct buckets) + alloc_size * sizeof(PyObject *));
    if (new_buckets == NULL) {
        PyErr_NoMemory();
        return -1;
    }
    new_buckets->size = new_size;
    new_buckets->nelem = 0;
    for (npy_intp i = 0; i < prev_size; i++) {
        PyObject **item = &old_buckets->array[i * (key_len + 1)];
        if (item[0] != NULL) {
            PyObject **tb_item = find_item_buckets(new_buckets, key_len, item + 1, NULL);
            memcpy(tb_item+1, item+1, key_len * sizeof(PyObject *));
            new_buckets->nelem++;
            tb_item[0] = item[0];
        }
    }
#ifdef Py_GIL_DISABLED
    new_buckets->prev = old_buckets;
#else
    PyMem_Free(old_buckets);
#endif
    FT_ATOMIC_STORE_PTR_RELEASE(tb->buckets, new_buckets);

Review comment (suggestion):
We removed the unused ability to replace items and this is leftover from that, so it's totally safe to delete

Code after:
}


static int
_resize_if_necessary(PyArrayIdentityHash *tb)
{
#ifdef Py_GIL_DISABLED
    assert(PyMutex_IsLocked(&tb->mutex));
#endif
    struct buckets *old_buckets = tb->buckets;
    int key_len = tb->key_len;
    npy_intp prev_size = old_buckets->size;
    assert(prev_size > 0);

    if ((old_buckets->nelem + 1) * 2 <= old_buckets->size) {
        /* No resize necessary if load factor is not more than 0.5 */
        return 0;
    }

    /* Double in size */
    npy_intp new_size = old_buckets->size * 2;

    npy_intp alloc_size;
    if (npy_mul_sizes_with_overflow(&alloc_size, new_size, key_len + 1)) {
        return -1;
    }
    struct buckets *new_buckets = (struct buckets *)PyMem_Calloc(
        1, sizeof(struct buckets) + alloc_size * sizeof(PyObject *));
    if (new_buckets == NULL) {
        PyErr_NoMemory();
        return -1;
    }
    new_buckets->size = new_size;
    new_buckets->nelem = 0;
    for (npy_intp i = 0; i < prev_size; i++) {
        PyObject **item = &old_buckets->array[i * (key_len + 1)];
        if (item[0] != NULL) {
            PyObject **tb_item = find_item_buckets(new_buckets, key_len, item + 1, NULL);
            memcpy(tb_item+1, item+1, key_len * sizeof(PyObject *));
            new_buckets->nelem++;
            tb_item[0] = item[0];
        }
    }
#ifdef Py_GIL_DISABLED
    new_buckets->prev = old_buckets;
#else
    PyMem_Free(old_buckets);
#endif
    FT_ATOMIC_STORE_PTR_RELEASE(tb->buckets, new_buckets);
    return 0;
}
Repository: numpy/numpy
File: numpy/_core/tests/test_multithreading.py (Python)

Code before:
            with concurrent.futures.ThreadPoolExecutor(max_workers=5) as tpe:
                tasks = [tpe.submit(read_arrs, b) for _ in range(4)]
                tasks.append(tpe.submit(mutation_func, b))
                for t in tasks:
                    t.result()
        except RuntimeError as e:
            if outcome == "success":
                raise
            assert "Inconsistent object during array creation?" in str(e)
            msg = "replace_list_items should not raise errors"
            assert mutation_func is contract_and_expand_list, msg
        finally:
            if len(tasks) < 5:
                b.abort()

def test_array__buffer__thread_safety():
    import inspect
    arr = np.arange(1000)
    flags = [inspect.BufferFlags.STRIDED, inspect.BufferFlags.READ]

    def func(b):
        b.wait()
        for i in range(100):
            arr.__buffer__(flags[i % 2])

    run_threaded(func, max_workers=8, pass_barrier=True)


Review comment (performance):
Given the complexity of the code paths in the implementation to handle void data types, I think it makes sense to add a test here that uses np.void.

Maybe look at `numpy/_core/tests/test_scalarbuffer.py::TestScalarPEP3118` for some examples that create buffers from scalars, including void scalars and `numpy/_core/tests/test_multiarray.py::TestPEP3118Dtype` and `TestNewBufferProtocol` in the same file for some examples using this API with nontrivial void dtypes.

Code after:
            with concurrent.futures.ThreadPoolExecutor(max_workers=5) as tpe:
                tasks = [tpe.submit(read_arrs, b) for _ in range(4)]
                tasks.append(tpe.submit(mutation_func, b))
                for t in tasks:
                    t.result()
        except RuntimeError as e:
            if outcome == "success":
                raise
            assert "Inconsistent object during array creation?" in str(e)
            msg = "replace_list_items should not raise errors"
            assert mutation_func is contract_and_expand_list, msg
        finally:
            if len(tasks) < 5:
                b.abort()

def test_array__buffer__thread_safety():
    import inspect
    arr = np.arange(1000)
    flags = [inspect.BufferFlags.STRIDED, inspect.BufferFlags.READ]

    def func(b):
        b.wait()
        for i in range(100):
            arr.__buffer__(flags[i % 2])

    run_threaded(func, max_workers=8, pass_barrier=True)

def test_void_dtype__buffer__thread_safety():
    import inspect
    dt = np.dtype([('name', np.str_, 16), ('grades', np.float64, (2,))])
    x = np.array(('ndarray_scalar', (1.2, 3.0)), dtype=dt)[()]
    assert isinstance(x, np.void)
    flags = [inspect.BufferFlags.STRIDES, inspect.BufferFlags.READ]

    def func(b):
        b.wait()
        for i in range(100):
            x.__buffer__(flags[i % 2])

    run_threaded(func, max_workers=8, pass_barrier=True)