Skip to content

Commit ed93ec1

Browse files
ndgrigorianndgrigorian
authored
Adds and implements utilities for validating array size and writable flag (#1547)
* Adds writable flag checks throughout Python bindings These checks are implemented through a new CheckWritable struct, which has a static method throw_if_not_writable, which throws if an array is read-only * Factors checks for sufficient memory range into output_validation.hpp * Corrects typo in py_binary_inplace_ufunc The check for sufficiently ample memory was performed with the incorrect operand
1 parent 275138b commit ed93ec1

21 files changed

+197
-333
lines changed

‎dpctl/apis/include/dpctl4pybind11.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ bool queues_are_compatible(const sycl::queue &exec_q,
10721072
return true;
10731073
}
10741074

1075-
/*! @brief Check if all allocation queues of usm_ndarays are the same as
1075+
/*! @brief Check if all allocation queues of usm_ndarays are the same as
10761076
the execution queue */
10771077
template <std::size_t num>
10781078
bool queues_are_compatible(const sycl::queue &exec_q,

‎dpctl/tensor/libtensor/include/utils/output_validation.hpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//===- output_validation.hpp - Utilities for output array validation
2+
//-*-C++-*===//
3+
//
4+
// Data Parallel Control (dpctl)
5+
//
6+
// Copyright 2020-2022 Intel Corporation
7+
//
8+
// Licensed under the Apache License, Version 2.0 (the "License");
9+
// you may not use this file except in compliance with the License.
10+
// You may obtain a copy of the License at
11+
//
12+
// http://www.apache.org/licenses/LICENSE-2.0
13+
//
14+
// Unless required by applicable law or agreed to in writing, software
15+
// distributed under the License is distributed on an "AS IS" BASIS,
16+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
// See the License for the specific language governing permissions and
18+
// limitations under the License.
19+
//
20+
//===----------------------------------------------------------------------===//
21+
///
22+
/// \file
23+
/// This file defines utilities for determining if an array is a valid output
24+
/// array.
25+
//===----------------------------------------------------------------------===//
26+
27+
#pragma once
28+
#include "dpctl4pybind11.hpp"
29+
#include <pybind11/pybind11.h>
30+
31+
namespace dpctl
32+
{
33+
34+
namespace tensor
35+
{
36+
37+
namespace validation
38+
{
39+
40+
/*! @brief Raises a value error if an array is read-only.
41+
42+
This should be called with an array before writing.*/
43+
struct CheckWritable
44+
{
45+
static void throw_if_not_writable(const dpctl::tensor::usm_ndarray &arr)
46+
{
47+
if (!arr.is_writable()) {
48+
throw py::value_error("output array is read-only.");
49+
}
50+
return;
51+
}
52+
};
53+
54+
/*! @brief Raises a value error if an array's memory is not sufficiently ample
55+
to accommodate an input number of elements.
56+
57+
This should be called with an array before writing.*/
58+
struct AmpleMemory
59+
{
60+
template <typename T>
61+
static void throw_if_not_ample(const dpctl::tensor::usm_ndarray &arr,
62+
T nelems)
63+
{
64+
auto arr_offsets = arr.get_minmax_offsets();
65+
T range = static_cast<T>(arr_offsets.second - arr_offsets.first);
66+
if (range + 1 < nelems) {
67+
throw py::value_error("Memory addressed by the output array is not "
68+
"sufficiently ample.");
69+
}
70+
return;
71+
}
72+
};
73+
74+
} // namespace validation
75+
} // namespace tensor
76+
} // namespace dpctl

‎dpctl/tensor/libtensor/source/accumulators.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "simplify_iteration_space.hpp"
3636
#include "utils/memory_overlap.hpp"
3737
#include "utils/offset_utils.hpp"
38+
#include "utils/output_validation.hpp"
3839
#include "utils/type_dispatch.hpp"
3940

4041
namespace dpctl
@@ -102,6 +103,8 @@ size_t py_mask_positions(const dpctl::tensor::usm_ndarray &mask,
102103
sycl::queue &exec_q,
103104
const std::vector<sycl::event> &depends)
104105
{
106+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(cumsum);
107+
105108
// cumsum is 1D
106109
if (cumsum.get_ndim() != 1) {
107110
throw py::value_error("Result array must be one-dimensional.");
@@ -274,6 +277,8 @@ size_t py_cumsum_1d(const dpctl::tensor::usm_ndarray &src,
274277
"Execution queue is not compatible with allocation queues");
275278
}
276279

280+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(cumsum);
281+
277282
if (src_size == 0) {
278283
return 0;
279284
}

‎dpctl/tensor/libtensor/source/boolean_advanced_indexing.cpp

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "simplify_iteration_space.hpp"
3838
#include "utils/memory_overlap.hpp"
3939
#include "utils/offset_utils.hpp"
40+
#include "utils/output_validation.hpp"
4041
#include "utils/type_dispatch.hpp"
4142

4243
namespace dpctl
@@ -118,6 +119,8 @@ py_extract(const dpctl::tensor::usm_ndarray &src,
118119
sycl::queue &exec_q,
119120
const std::vector<sycl::event> &depends)
120121
{
122+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
123+
121124
int src_nd = src.get_ndim();
122125
if ((axis_start < 0 || axis_end > src_nd || axis_start >= axis_end)) {
123126
throw py::value_error("Specified axes_start and axes_end are invalid.");
@@ -171,19 +174,8 @@ py_extract(const dpctl::tensor::usm_ndarray &src,
171174
throw py::value_error("Inconsistent array dimensions");
172175
}
173176

174-
// ensure that dst is sufficiently ample
175-
auto dst_offsets = dst.get_minmax_offsets();
176-
// destination must be ample enough to accommodate all elements
177-
{
178-
size_t range =
179-
static_cast<size_t>(dst_offsets.second - dst_offsets.first);
180-
if (range + 1 < static_cast<size_t>(ortho_nelems * masked_dst_nelems)) {
181-
throw py::value_error(
182-
"Memory addressed by the destination array can not "
183-
"accommodate all the "
184-
"array elements.");
185-
}
186-
}
177+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(
178+
dst, ortho_nelems * masked_dst_nelems);
187179

188180
auto const &overlap = dpctl::tensor::overlap::MemoryOverlap();
189181
// check that dst does not intersect with src, not with cumsum.
@@ -452,6 +444,8 @@ py_place(const dpctl::tensor::usm_ndarray &dst,
452444
sycl::queue &exec_q,
453445
const std::vector<sycl::event> &depends)
454446
{
447+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
448+
455449
int dst_nd = dst.get_ndim();
456450
if ((axis_start < 0 || axis_end > dst_nd || axis_start >= axis_end)) {
457451
throw py::value_error("Specified axes_start and axes_end are invalid.");
@@ -502,19 +496,8 @@ py_place(const dpctl::tensor::usm_ndarray &dst,
502496
throw py::value_error("Inconsistent array dimensions");
503497
}
504498

505-
// ensure that dst is sufficiently ample
506-
auto dst_offsets = dst.get_minmax_offsets();
507-
// destination must be ample enough to accommodate all elements
508-
{
509-
size_t range =
510-
static_cast<size_t>(dst_offsets.second - dst_offsets.first);
511-
if (range + 1 < static_cast<size_t>(ortho_nelems * masked_dst_nelems)) {
512-
throw py::value_error(
513-
"Memory addressed by the destination array can not "
514-
"accommodate all the "
515-
"array elements.");
516-
}
517-
}
499+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(
500+
dst, ortho_nelems * masked_dst_nelems);
518501

519502
auto const &overlap = dpctl::tensor::overlap::MemoryOverlap();
520503
// check that dst does not intersect with src, not with cumsum.
@@ -726,6 +709,8 @@ py_nonzero(const dpctl::tensor::usm_ndarray
726709
"Execution queue is not compatible with allocation queues");
727710
}
728711

712+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(indexes);
713+
729714
int cumsum_nd = cumsum.get_ndim();
730715
if (cumsum_nd != 1 || !cumsum.is_c_contiguous()) {
731716
throw py::value_error("Cumsum array must be a C-contiguous vector");
@@ -787,18 +772,8 @@ py_nonzero(const dpctl::tensor::usm_ndarray
787772
throw py::value_error("Arrays are expected to ave no memory overlap");
788773
}
789774

790-
// ensure that dst is sufficiently ample
791-
auto indexes_offsets = indexes.get_minmax_offsets();
792-
// destination must be ample enough to accommodate all elements
793-
{
794-
size_t range =
795-
static_cast<size_t>(indexes_offsets.second - indexes_offsets.first);
796-
if (range + 1 < static_cast<size_t>(nz_elems * _ndim)) {
797-
throw py::value_error(
798-
"Memory addressed by the destination array can not "
799-
"accommodate all the array elements.");
800-
}
801-
}
775+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(
776+
indexes, nz_elems * _ndim);
802777

803778
std::vector<sycl::event> host_task_events;
804779
host_task_events.reserve(2);

‎dpctl/tensor/libtensor/source/clip.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "simplify_iteration_space.hpp"
3838
#include "utils/memory_overlap.hpp"
3939
#include "utils/offset_utils.hpp"
40+
#include "utils/output_validation.hpp"
4041
#include "utils/type_dispatch.hpp"
4142

4243
namespace dpctl
@@ -87,6 +88,8 @@ py_clip(const dpctl::tensor::usm_ndarray &src,
8788
"Execution queue is not compatible with allocation queues");
8889
}
8990

91+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
92+
9093
int nd = src.get_ndim();
9194
int min_nd = min.get_ndim();
9295
int max_nd = max.get_ndim();
@@ -152,19 +155,7 @@ py_clip(const dpctl::tensor::usm_ndarray &src,
152155
"have the same data type");
153156
}
154157

155-
// ensure that dst is sufficiently ample
156-
auto dst_offsets = dst.get_minmax_offsets();
157-
// destination must be ample enough to accommodate all elements
158-
{
159-
size_t range =
160-
static_cast<size_t>(dst_offsets.second - dst_offsets.first);
161-
if (range + 1 < static_cast<size_t>(nelems)) {
162-
throw py::value_error(
163-
"Memory addressed by the destination array can not "
164-
"accommodate all the "
165-
"array elements.");
166-
}
167-
}
158+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(dst, nelems);
168159

169160
char *src_data = src.get_data();
170161
char *min_data = min.get_data();

‎dpctl/tensor/libtensor/source/copy_and_cast_usm_to_usm.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "dpctl4pybind11.hpp"
3838
#include "kernels/copy_and_cast.hpp"
3939
#include "utils/memory_overlap.hpp"
40+
#include "utils/output_validation.hpp"
4041
#include "utils/type_dispatch.hpp"
4142
#include "utils/type_utils.hpp"
4243

@@ -100,24 +101,16 @@ copy_usm_ndarray_into_usm_ndarray(const dpctl::tensor::usm_ndarray &src,
100101
return std::make_pair(sycl::event(), sycl::event());
101102
}
102103

103-
// destination must be ample enough to accommodate all elements
104-
{
105-
auto dst_offsets = dst.get_minmax_offsets();
106-
size_t range =
107-
static_cast<size_t>(dst_offsets.second - dst_offsets.first);
108-
if (range + 1 < src_nelems) {
109-
throw py::value_error(
110-
"Destination array can not accommodate all the "
111-
"elements of source array.");
112-
}
113-
}
104+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(dst, src_nelems);
114105

115106
// check compatibility of execution queue and allocation queue
116107
if (!dpctl::utils::queues_are_compatible(exec_q, {src, dst})) {
117108
throw py::value_error(
118109
"Execution queue is not compatible with allocation queues");
119110
}
120111

112+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
113+
121114
int src_typenum = src.get_typenum();
122115
int dst_typenum = dst.get_typenum();
123116

‎dpctl/tensor/libtensor/source/copy_for_reshape.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "copy_for_reshape.hpp"
3030
#include "dpctl4pybind11.hpp"
3131
#include "kernels/copy_and_cast.hpp"
32+
#include "utils/output_validation.hpp"
3233
#include "utils/type_dispatch.hpp"
3334
#include <pybind11/pybind11.h>
3435

@@ -87,24 +88,16 @@ copy_usm_ndarray_for_reshape(const dpctl::tensor::usm_ndarray &src,
8788
return std::make_pair(sycl::event(), sycl::event());
8889
}
8990

90-
// destination must be ample enough to accommodate all elements
91-
{
92-
auto dst_offsets = dst.get_minmax_offsets();
93-
py::ssize_t range =
94-
static_cast<py::ssize_t>(dst_offsets.second - dst_offsets.first);
95-
if (range + 1 < src_nelems) {
96-
throw py::value_error(
97-
"Destination array can not accommodate all the "
98-
"elements of source array.");
99-
}
100-
}
91+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(dst, src_nelems);
10192

10293
// check same contexts
10394
if (!dpctl::utils::queues_are_compatible(exec_q, {src, dst})) {
10495
throw py::value_error(
10596
"Execution queue is not compatible with allocation queues");
10697
}
10798

99+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
100+
108101
if (src_nelems == 1) {
109102
// handle special case of 1-element array
110103
int src_elemsize = src.get_elemsize();

‎dpctl/tensor/libtensor/source/copy_for_roll.cpp

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "copy_for_roll.hpp"
3030
#include "dpctl4pybind11.hpp"
3131
#include "kernels/copy_and_cast.hpp"
32+
#include "utils/output_validation.hpp"
3233
#include "utils/type_dispatch.hpp"
3334
#include <pybind11/pybind11.h>
3435

@@ -110,24 +111,16 @@ copy_usm_ndarray_for_roll_1d(const dpctl::tensor::usm_ndarray &src,
110111
return std::make_pair(sycl::event(), sycl::event());
111112
}
112113

113-
// destination must be ample enough to accommodate all elements
114-
{
115-
auto dst_offsets = dst.get_minmax_offsets();
116-
py::ssize_t range =
117-
static_cast<py::ssize_t>(dst_offsets.second - dst_offsets.first);
118-
if (range + 1 < src_nelems) {
119-
throw py::value_error(
120-
"Destination array can not accommodate all the "
121-
"elements of source array.");
122-
}
123-
}
114+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(dst, src_nelems);
124115

125116
// check same contexts
126117
if (!dpctl::utils::queues_are_compatible(exec_q, {src, dst})) {
127118
throw py::value_error(
128119
"Execution queue is not compatible with allocation queues");
129120
}
130121

122+
dpctl::tensor::validation::CheckWritable::throw_if_not_writable(dst);
123+
131124
if (src_nelems == 1) {
132125
// handle special case of 1-element array
133126
int src_elemsize = src.get_elemsize();
@@ -298,17 +291,7 @@ copy_usm_ndarray_for_roll_nd(const dpctl::tensor::usm_ndarray &src,
298291
return std::make_pair(sycl::event(), sycl::event());
299292
}
300293

301-
// destination must be ample enough to accommodate all elements
302-
{
303-
auto dst_offsets = dst.get_minmax_offsets();
304-
py::ssize_t range =
305-
static_cast<py::ssize_t>(dst_offsets.second - dst_offsets.first);
306-
if (range + 1 < src_nelems) {
307-
throw py::value_error(
308-
"Destination array can not accommodate all the "
309-
"elements of source array.");
310-
}
311-
}
294+
dpctl::tensor::validation::AmpleMemory::throw_if_not_ample(dst, src_nelems);
312295

313296
// check for compatible queues
314297
if (!dpctl::utils::queues_are_compatible(exec_q, {src, dst})) {

0 commit comments

Comments
 (0)