Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encapsulate Storage_base class #528

Open
wants to merge 4 commits into
base: dev-master
Choose a base branch
from
Open

Encapsulate Storage_base class #528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
562f093
Implement Storage::release()
IvanaGyro Dec 1, 2024
ad6610f
Do not assign the value to the variable of Stroage
IvanaGyro Dec 1, 2024
602507b
Encapsulate Storage_base class
IvanaGyro Dec 1, 2024
09b2c37
Remove unsed debug message
IvanaGyro Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/Tensor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@
"[ERROR] Attempt to convert dtype %d (%s) to pointer of type %s",
this->dtype(), Type_class::getname(this->dtype()).c_str(),
Type_class::getname(Type_class::cy_typeid_v<std::remove_cv_t<T>>).c_str());
return static_cast<T *>(this->_impl->_storage._impl->Mem);
return static_cast<T *>(this->_impl->_storage._impl->data());

Check warning on line 524 in include/Tensor.hpp

View check run for this annotation

Codecov / codecov/patch

include/Tensor.hpp#L524 

Added line #L524 was not covered by tests
}

#ifdef UNI_GPU
Expand All @@ -542,7 +542,7 @@
"[ERROR] Attempt to convert dtype %d (%s) to GPU pointer of type %s", this->dtype(),
Type_class::getname(this->dtype()).c_str(),
Type_class::getname(Type_class::cy_typeid_gpu_v<std::remove_cv_t<T>>).c_str());
return static_cast<T *>(this->_impl->_storage._impl->Mem);
return static_cast<T *>(this->_impl->_storage._impl->data());
}
#endif

Expand Down Expand Up @@ -1517,7 +1517,7 @@
this->_impl->_storage.resize(oldsize + in.size());
memcpy(((char *)this->_impl->_storage.data()) +
oldsize * Type.typeSize(this->dtype()) / sizeof(char),
in._impl->Mem, Type.typeSize(in.dtype()) * in.size());
in._impl->data(), Type.typeSize(in.dtype()) * in.size());
}
/*
void append(const Tensor &rhs){
Expand Down
3 changes: 3 additions & 0 deletions include/backend/Scalar.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ namespace cytnx {
return nullptr;
}

/**
* @deprecated This method is not in use anymore.
*/
virtual void *get_raw_address() const {
cytnx_error_msg(true, "[ERROR] Void Type Scalar cannot have operation!!%s", "\n");
return nullptr;
Copy link
Collaborator Author

@IvanaGyro IvanaGyro Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark this function as deprecated. It may be deleted or renamed to data() like std convention.

Expand Down
140 changes: 95 additions & 45 deletions include/backend/Storage.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,7 @@
///@cond
class Storage_base : public intrusive_ptr_base<Storage_base> {
public:
void *Mem;
// std::vector<unsigned int> shape;

unsigned long long len; // default 0
unsigned long long cap; // default 0
unsigned int dtype; // default 0, Void
int device; // default -1, on cpu

Storage_base() : cap(0), len(0), Mem(NULL), dtype(0), device(-1){};
Storage_base() = default;
// Storage_base(const std::initializer_list<unsigned int> &init_shape);
// Storage_base(const std::vector<unsigned int> &init_shape);
Storage_base(const unsigned long long &len_in, const int &device, const bool &init_zero = true);
Expand All @@ -46,9 +38,13 @@
// void Init(const std::initializer_list<unsigned int> &init_shape);
std::string dtype_str() const;
std::string device_str() const;
const unsigned long long &capacity() const { return this->cap; }
const unsigned long long &size() const { return this->len; }
~Storage_base();
virtual const unsigned long long capacity() const {
cytnx_error_msg(true, "Not implemented.%s", "");

Check warning on line 42 in include/backend/Storage.hpp

View check run for this annotation

Codecov / codecov/patch

include/backend/Storage.hpp#L41-L42 

Added lines #L41 - L42 were not covered by tests
}
virtual const unsigned long long size() const {
cytnx_error_msg(true, "Not implemented.%s", "");

Check warning on line 45 in include/backend/Storage.hpp

View check run for this annotation

Codecov / codecov/patch

include/backend/Storage.hpp#L44-L45 

Added lines #L44 - L45 were not covered by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to return a const reference for the ScalarType.

}
virtual ~Storage_base();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base classes must have a virtual deconstructor. Without virtual, the deconstructor will not be called when deleting the instance of the derived class held by the pointer of the base classes.


template <class T>
T &at(const cytnx_uint64 &idx) const;
Expand All @@ -59,7 +55,11 @@
template <class T>
T *data() const;

void *data() const { return this->Mem; }
// `Storage_base` can be instanitiated directly. It's deconstructor calls `data()`, so we cannot
// throw a runtime error here.
virtual void *data() const { return nullptr; }
virtual int dtype() const { return Type.Void; }
virtual int device() const { return Device.cpu; }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New getters due to the variables of Storage_base are moved to the derived class.


void _cpy_bool(void *ptr, const std::vector<cytnx_bool> &vin);

Expand Down Expand Up @@ -87,37 +87,37 @@
template <class T>
void _Init_byptr_safe(T *rawptr, const unsigned long long &len_in) {
// check:
if (this->dtype == Type.Float) {
if (this->dtype() == Type.Float) {
cytnx_error_msg(typeid(T) != typeid(cytnx_float), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Double) {
} else if (this->dtype() == Type.Double) {
cytnx_error_msg(typeid(T) != typeid(cytnx_double), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Uint64) {
} else if (this->dtype() == Type.Uint64) {
cytnx_error_msg(typeid(T) != typeid(cytnx_uint64), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Uint32) {
} else if (this->dtype() == Type.Uint32) {
cytnx_error_msg(typeid(T) != typeid(cytnx_uint32), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Int64) {
} else if (this->dtype() == Type.Int64) {
cytnx_error_msg(typeid(T) != typeid(cytnx_int64), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Int32) {
} else if (this->dtype() == Type.Int32) {
cytnx_error_msg(typeid(T) != typeid(cytnx_int32), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.ComplexDouble) {
} else if (this->dtype() == Type.ComplexDouble) {
cytnx_error_msg(typeid(T) != typeid(cytnx_complex128), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.ComplexFloat) {
} else if (this->dtype() == Type.ComplexFloat) {
cytnx_error_msg(typeid(T) != typeid(cytnx_complex64), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Int16) {
} else if (this->dtype() == Type.Int16) {
cytnx_error_msg(typeid(T) != typeid(cytnx_int16), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Uint16) {
} else if (this->dtype() == Type.Uint16) {
cytnx_error_msg(typeid(T) != typeid(cytnx_uint16), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else if (this->dtype == Type.Bool) {
} else if (this->dtype() == Type.Bool) {
cytnx_error_msg(typeid(T) != typeid(cytnx_bool), "%s",
"[ERROR _Init_byptr_safe type not match]");
} else {
Expand Down Expand Up @@ -146,6 +146,18 @@
const std::vector<cytnx_uint64> &shape,
const std::vector<std::vector<cytnx_uint64>> &locators,
const cytnx_uint64 &Nunit, const bool &is_scalar);

/**
* @brief Drop the ownership of the underlying contiguous memory.
*
* The caller MUST take the ownership before calling this method. Any operation following this
* method causes undefined behavior.
*
* @return The pointer referencing the underlying storage.
* @deprecated This method may be removed without any notification.
*/
virtual void *release() noexcept { return nullptr; }

Check warning on line 159 in include/backend/Storage.hpp

View check run for this annotation

Codecov / codecov/patch

include/backend/Storage.hpp#L159 

Added line #L159 was not covered by tests

// these is the one that do the work, and customize with Storage_base
// virtual void Init(const std::vector<unsigned int> &init_shape);
virtual void Init(const unsigned long long &len_in, const int &device = -1,
Expand Down Expand Up @@ -219,17 +231,15 @@
virtual void set_item(const cytnx_uint64 &idx, const cytnx_int16 &val);
virtual void set_item(const cytnx_uint64 &idx, const cytnx_uint16 &val);
virtual void set_item(const cytnx_uint64 &idx, const cytnx_bool &val);

// virtual bool approx_eq(const boost::intrusive_ptr<Storage_base> &rhs,
// const cytnx_double tol = 1e-8);
};
///@endcond

///@cond
template <typename DType>
class StorageImplementation : public Storage_base {
public:
StorageImplementation() { this->dtype = Type.cy_typeid(DType()); };
StorageImplementation()
: capacity_(0), size_(0), start_(nullptr), dtype_(Type.cy_typeid(DType())), device_(-1){};
void Init(const unsigned long long &len_in, const int &device = -1,
const bool &init_zero = true);
void _Init_byptr(void *rawptr, const unsigned long long &len_in, const int &device = -1,
Expand All @@ -251,6 +261,29 @@
boost::intrusive_ptr<Storage_base> real();
boost::intrusive_ptr<Storage_base> imag();

const unsigned long long capacity() const override { return capacity_; }
const unsigned long long size() const override { return size_; }
void *data() const override { return start_; }
int dtype() const override { return dtype_; }
int device() const override { return device_; }

/**
* @brief Drop the ownership of the underlying contiguous memory.
*
* The caller MUST take the ownership before calling this method. Any operation following this
* method causes undefined behavior.
*
* @return The pointer referencing the underlying storage.
* @deprecated This method may be removed without any notification.
*/
void *release() noexcept override {
void *original_start = start_;
start_ = nullptr;
size_ = 0;
capacity_ = 0;
return original_start;

Check warning on line 284 in include/backend/Storage.hpp

View check run for this annotation

Codecov / codecov/patch

include/backend/Storage.hpp#L279-L284 

Added lines #L279 - L284 were not covered by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be used in the functions converting Tensor and Storage to numpy arrays.

};

// generators:
void fill(const cytnx_complex128 &val);
void fill(const cytnx_complex64 &val);
Expand Down Expand Up @@ -304,6 +337,12 @@
template <typename OtherDType>
void SetItem(cytnx_uint64 index, const OtherDType &value);
void SetItem(cytnx_uint64 index, const Scalar &value);

void *start_;
unsigned long long size_;
unsigned long long capacity_;
unsigned int dtype_;
int device_;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables moved from Storage_base class to its derived class, StorageImplementation<DType>.

};
///@endcond

Expand Down Expand Up @@ -583,7 +622,7 @@
@brief the dtype-id of current Storage, see cytnx::Type for more details.
@return [cytnx_uint64] the dtype-id.
*/
const unsigned int &dtype() const { return this->_impl->dtype; }
unsigned int dtype() const { return this->_impl->dtype(); }

/**
@brief the dtype (std::string) of current Storage, see cytnx::Type for more details.
Expand All @@ -597,7 +636,7 @@
@brief the device-id of current Storage, see cytnx::Device for more details.
@return [cytnx_int64] the device-id.
*/
const int &device() const { return this->_impl->device; }
int device() const { return this->_impl->device(); }

/**
@brief the device (std::string) of current Storage, see cytnx::Device for more details.
Expand Down Expand Up @@ -699,7 +738,7 @@
@return [cytnx_uint64]

*/
const unsigned long long &size() const { return this->_impl->len; }
unsigned long long size() const { return this->_impl->size(); }

/**
@brief the capacity in the Storage.
Expand All @@ -708,7 +747,18 @@
@return [cytnx_uint64]

*/
const unsigned long long &capacity() const { return this->_impl->cap; }
unsigned long long capacity() const { return this->_impl->capacity(); }

/**
* @brief Drop the ownership of the underlying contiguous memory.
*
* The caller MUST take the ownership before calling this method. Any operation following this
* method causes undefined behavior.
*
* @return The pointer referencing the underlying storage.
* @deprecated This method may be removed without any notification.
*/
void *release() noexcept { return this->_impl->release(); }

Check warning on line 761 in include/backend/Storage.hpp

View check run for this annotation

Codecov / codecov/patch

include/backend/Storage.hpp#L761 

Added line #L761 was not covered by tests

/**
@brief print the info of the Storage, including the device, dtype and size.
Expand Down Expand Up @@ -818,64 +868,64 @@
// check:
cytnx_error_msg(1, "[FATAL] ERROR unsupport type%s", "\n");
// this->_impl->Init(vin.size(),device);
// memcpy(this->_impl->Mem,&vin[0],sizeof(T)*vin.size());
// memcpy(this->_impl->data(),&vin[0],sizeof(T)*vin.size());
}

void _from_vector(const std::vector<cytnx_complex128> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.ComplexDouble]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_complex128) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_complex128) * vin.size());
}
void _from_vector(const std::vector<cytnx_complex64> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.ComplexFloat]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_complex64) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_complex64) * vin.size());
}
void _from_vector(const std::vector<cytnx_double> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Double]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_double) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_double) * vin.size());
}
void _from_vector(const std::vector<cytnx_float> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Float]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_float) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_float) * vin.size());
}
void _from_vector(const std::vector<cytnx_uint64> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Uint64]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_uint64) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_uint64) * vin.size());
}
void _from_vector(const std::vector<cytnx_int64> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Int64]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_int64) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_int64) * vin.size());
}
void _from_vector(const std::vector<cytnx_uint32> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Uint32]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_uint32) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_uint32) * vin.size());
}
void _from_vector(const std::vector<cytnx_int32> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Int32]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_int32) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_int32) * vin.size());
}
void _from_vector(const std::vector<cytnx_uint16> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Uint16]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_uint16) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_uint16) * vin.size());
}
void _from_vector(const std::vector<cytnx_int16> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Int16]();
this->_impl->Init(vin.size(), device);
memcpy(this->_impl->Mem, &vin[0], sizeof(cytnx_int16) * vin.size());
memcpy(this->_impl->data(), &vin[0], sizeof(cytnx_int16) * vin.size());
}
void _from_vector(const std::vector<cytnx_bool> &vin, const int device = -1) {
this->_impl = __SII.USIInit[Type.Bool]();
this->_impl->Init(vin.size(), device);
this->_impl->_cpy_bool(this->_impl->Mem, vin);
// memcpy(this->_impl->Mem,vin.data(),sizeof(cytnx_bool)*vin.size());
this->_impl->_cpy_bool(this->_impl->data(), vin);
// memcpy(this->_impl->data(),vin.data(),sizeof(cytnx_bool)*vin.size());
}
/// @endcond

Expand Down
2 changes: 1 addition & 1 deletion pybind/generator_py.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void generator_binding(py::module &m) {

Tensor m;
m.Init(shape, dtype);
memcpy(m.storage()._impl->Mem, info.ptr, Totbytes);
memcpy(m.storage()._impl->data(), info.ptr, Totbytes);
return m;
});
}
Expand Down
23 changes: 8 additions & 15 deletions pybind/storage_py.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <vector>
#include <map>
#include <random>
#include <string>
#include <vector>

#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
Expand Down Expand Up @@ -34,9 +35,9 @@ void storage_binding(py::module &m) {
}

// calculate stride:
std::vector<ssize_t> stride(1, Type.typeSize(tmpIN.dtype()));
size_t type_size = Type.typeSize(tmpIN.dtype());
std::vector<ssize_t> stride(1, type_size);
std::vector<ssize_t> shape(1, tmpIN.size());
// ssize_t accu = tmpIN.size();

py::buffer_info npbuf;
std::string chr_dtype;
Expand All @@ -63,19 +64,11 @@ void storage_binding(py::module &m) {
"\n");
}

npbuf = py::buffer_info(tmpIN._impl->Mem, // ptr
Type.typeSize(tmpIN.dtype()), // size of elem
// Call `.release()` to avoid the memory passed to numpy being freed.
npbuf = py::buffer_info(tmpIN.release(), type_size,
chr_dtype, // pss format
1, // rank
shape, // shape
stride // stride
);
py::array out(npbuf);
// delegate numpy array with it's ptr, and swap a auxiliary ptr for intrusive_ptr to
// free.
void *pswap = malloc(sizeof(bool));
tmpIN._impl->Mem = pswap;
return out;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables should be private. release() is designed to provide the same functionality.

/* rank= */ 1, shape, stride);
return py::array(npbuf);
})

// construction
Expand Down
Loading
Loading