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.

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

Unsafe access to accumulator data #323

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

HDembinski
Copy link
Collaborator

@HDembinski HDembinski commented Apr 27, 2021

  • Test this with downstream boost-histogram

@HDembinski
Copy link
Collaborator Author

@henryiii Before I change all accumulators in this way, we should try first whether this change works the way we expect in boost-histogram. I can try to make the changes to test this in boost-histogram.

@henryiii
Copy link
Contributor

Can this be rebased on top of current develop?

@HDembinski HDembinski force-pushed the unsafe_access_accumulators branch from db6e4c3 to 598a51f Compare September 26, 2021 10:32
@HDembinski
Copy link
Collaborator Author

@henryiii Rebased on develop as you requested

@HDembinski HDembinski force-pushed the unsafe_access_accumulators branch from 598a51f to 6a5af14 Compare September 30, 2021 09:56
@henryiii
Copy link
Contributor

Thanks, I'll work on this further soon - might be after Python 3.10 / pybind11 2.8 / Windows 11 though, all to be released next Monday. (I don't think Windows 11 affects me, but recently noticed it had the same release date as Python 3.10) First initial try showed it will take some work, but didn't get far enough for good feedback yet.

@HDembinski HDembinski force-pushed the develop branch 3 times, most recently from 90ebbe0 to bf7712f Compare November 15, 2021 08:29
@henryiii
Copy link
Contributor

henryiii commented Feb 8, 2022

FYI, I did attempt this a little while ago, but it will take a bit of work to properly implement. Boost-histogram has to know the memory layout exactly, since it can cast between a block of Python memory to C++ without copy. Convincing pybind11 of this without access to the actual structure just requires manual work for things normally covered by macros. I'll try to get back to this in ~1 week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants