Fix broadcasting on read #136
Conversation
|
This is not necessarily the only possible or best solution, though it is a solution for the cases I’ve tested. I can verify in a larger code base that it fixes the odd C-api segfaults when you don’t read the whole array many times. I’m not sure why this would be a C-api segfault, but it made it very hard to track down. |
|
Is this a reasonable fix, or is something else better? Currently, HighFive cannot read files with scalars and 1D arrays created with a pretty normal setup with the old C++ HDF5 bindings, and the missmatch between the HighFive checker and reader causes segfaults instead of throwing an error. |
|
@henryiii Looks reasonable at a first glance, but I would like to understand better the problem myself before deciding whether this is the best solution at the moment or not. Regretfully I didn't have to look into it last week. |
While DataSpace::checkDimensions was collapsing dimensions with value 1, the same logic was not cosidered in data_converters for std::vector and std::array. Removed the unused dim parameter from all data_converters.
hernando
force-pushed
the
fixbroadcasting
branch
from
95f7053
to
49279f3
Compare
|
I've slightly modified your solution. In particular, I have removed the getFilteredDimensions method as filter is more generic than removing dimensions which are one. It was only used in the implementation so I prefer keeping it there. I also changed the code to not create a new dims vector and simply process the given one to get exactly what is needed and nothing more. |
|
Excellent, thank you! |
This is the test mentioned in #135, and a possible fix. Only fixes 1D vectors and arrays at the moment.
It works by giving the unnested vector and array converters a filtered version of the dimensions array, where all the 1s have been removed unless it is all 1s, in which case a single 1 is kept.
Higher dimensional arrays may still have issues where they don’t throw an error, but miss the correct non-1 dimensions.