Skip to content

Commit

Permalink
Fix instantiation of G3Timestreams from iterables.
Browse files Browse the repository at this point in the history
Under certain circumstances that I have not quite tracked down, Boost's
type inference for the stub push_back() failed, resulting in data
corruption. Instead, use a type it is designed to support (std::vector<double>)
and retire the stub push_back() hack. This incurs a memcpy() but the
overhead is tiny given that this is only called in circumstances that
involve a Python loop anyway.

Closes #93.
  • Loading branch information
nwhitehorn committed Nov 16, 2022
1 parent c45570b commit 96ee415
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 19 deletions.
14 changes: 0 additions & 14 deletions core/include/core/G3Timestream.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,6 @@ class G3Timestream : public G3FrameObject {
G3Timestream operator /(double r) const;
G3Timestream &operator /=(double r);

// Avoid using the following, it works only in very restricted cases
void push_back(double value) {
if (!buffer_) {
// Newly-allocated zero-length timestream?
if (!data_ && len_ == 0)
buffer_ = new std::vector<double>();
else
throw std::bad_alloc();
}
buffer_->push_back(value);
// Update pointers and length, which may have changed
data_ = &buffer_[0];
len_ = buffer_->size();
}
class G3TimestreamPythonHelpers;

private:
Expand Down
12 changes: 7 additions & 5 deletions core/src/G3Timestream.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ timestream_from_iterable(boost::python::object v,
G3TimestreamPtr x;
Py_buffer view;
if (PyObject_GetBuffer(v.ptr(), &view,
PyBUF_FORMAT | PyBUF_ANY_CONTIGUOUS) != -1) {
PyBUF_FORMAT | PyBUF_CONTIG_RO) != -1) {
if (strcmp(view.format, "d") == 0) {
x = G3TimestreamPtr(new G3Timestream((double *)view.buf,
(double *)view.buf + view.len/sizeof(double)));
Expand Down Expand Up @@ -961,13 +961,16 @@ timestream_from_iterable(boost::python::object v,
} else {
// We could add more types, but why do that?
// Let Python do the work for obscure cases
x = G3TimestreamPtr(new G3Timestream());
boost::python::container_utils::extend_container(*x, v);
std::vector<double> data;
boost::python::container_utils::extend_container(data, v);
x = G3TimestreamPtr(new G3Timestream(data.begin(), data.end()));
}
PyBuffer_Release(&view);
} else {
PyErr_Clear();
boost::python::container_utils::extend_container(*x, v);
std::vector<double> data;
boost::python::container_utils::extend_container(data, v);
x = G3TimestreamPtr(new G3Timestream(data.begin(), data.end()));
}

x->units = units;
Expand Down Expand Up @@ -1247,7 +1250,6 @@ PYBINDINGS("core") {
.def("_cxxslice", &G3Timestream::G3TimestreamPythonHelpers::G3Timestream_getslice, "Slice-only __getitem__")
// Operators bound in python through numpy
;
scitbx::boost_python::container_conversions::from_python_sequence<G3Timestream, scitbx::boost_python::container_conversions::variable_capacity_policy>();
register_pointer_conversions<G3Timestream>();

// Add buffer protocol interface
Expand Down

0 comments on commit 96ee415

Please sign in to comment.