NWChemEx / Chemist

Library for describing chemical systems
https://nwchemex.github.io/Chemist/
Apache License 2.0
1 stars 0 forks source link

Better Contiguous Memory Support #372

Closed ryanmrichard closed 5 months ago

ryanmrichard commented 1 year ago

Part of the reason for the complexity of the Nuclei, Molecule, and various basis set classes is to ensure that a "structure of arrays" can be used as an "array of structures". More fundamentally we want to rely on the underlying memory being contiguous. Right now most of the views are implemented in a manner which assumes they are aliasing contiguous memory. In turn, users can grab the address of the references returned by the view, and use those addresses as contiguous memory buffers. Unfortunately, I'm running into competing use cases where we want to use the view classes with memory that is non-contiguous.

My proposed solution is that instead of relying on PointView to always point to contiguous data, we should make the request for contiguous data explicit via data() members on the containers (this is what std::vector does for example). In practice, we'll need data() members for each piece of state.

This issue does not require rewriting the views (although they may eventually need to be PIMPL'ed to deal with their state being stored differently) aside from needing to update the documentation to remove guarantees that the data be contiguous.

As a concrete example. Given a PointSet<double> object points the current mechanism for getting a pointer to the 0-th point's x coordinate is auto px0 = &points[0].x(); We could then manipulate that pointer to get the i-th points' x coordinate via auto pxi = px0 + i;. Under the current issue's proposal, auto px0 = &points[0].x() would still be a pointer to the 0-th point's x-coordinate; however, the auto pxi = px0 + i; statement would no longer be guaranteed to be true (it could be true, but user's shouldn't count on it). Instead, if a user wanted to guarantee auto pxi = px0 + i; behavior they should get px0 by auto px0 = points.x_data();.

ryanmrichard commented 5 months ago

I think this was addressed when I added the views.