pace-neutrons / Horace

Horace is a suite of programs for the visualization and analysis of large datasets from time-of-flight neutron inelastic scattering spectrometers.
https://pace-neutrons.github.io/Horace/stable/
GNU General Public License v3.0
8 stars 5 forks source link

get_bytestream_from_obj results in failed test in recent Matlab versions #1052

Open tgperring opened 1 year ago

tgperring commented 1 year ago

Starting from either Matlab 2021b or 2022a there has been a change to the Matlab intrinsic function getByteStreamFromArray such that the output can have a changed number of bytes for a given input compared to earlier Matlab versions. This was found running test_get_revert_bytestream:test_obj_conversion in _test\test_serializers, which fails due to the number of bytes being different from the expected value. The test fails in R2022a and R2023a, but is fine in R2019b and R2021a on TGP's Win10 laptop. The failure is at the call to getByteStreamFromArray in the Herbert routine get_bytestream_from_obj. This function itself is rather odd as (i) it has a branch on two or more input arguments but does not pass any of the arguments to another routine or use them itself, and (ii) it has calls to a function byte_stream which does not exist within PACE or Matlab.

mducle commented 1 year ago

Are these functions (get_bytestream_from_obj and get_obj_from_bytestream) used anywhere except in the test?

Maybe they should just be removed?

As far as I can tell serializable converts stuff to plain structs rather than bytestreams... and getByteStreamFromArray / getArrayFromByteStream are undocumented functions anyway, so changes between Matlab releases are not unexpected...

getByteStreamFromArray is actually used by @unique_objects_container to compute a hash to check if two objects have the same data... but this: 1) doesn't need the actual implementation to be the same across all Matlab versions (just that it returns the same bytestream for the same data even if they are in two different addresses); and 2) could be replaced by a mex file if needed...

cmarooney-stfc commented 1 year ago

@cmarooney-stfc check tests, use as backup, unique_containers. Not used for serialize now.