search

BIC-MNI / minc-tools

Basic minc-tools from former minc repository
Other
29 stars 25 forks source link

avoid getting attributes for a dimension with no axis, like time axis #120

Closed jiax1994 closed 1 year ago

jiax1994 commented 1 year ago

In case of time dimension, the NO_AXIS has a value of -1, this lead to call an array with negative index of -1 with separations[in_vol->file->world_axes[idim_in]]. In C, an array with negative index would lead problems related to memory location's value being overwritten. So a solution is proposed to only get attribute for world dimensions, and skip for time dimension by adding continue with such condition.

This leads to issue typically with 4D input volumes with floating data type, when building in release mode with optimization.

gdevenyi commented 1 year ago

Can you provide an example file and test showing the bug and how things act after this patch?

gdevenyi commented 1 year ago

Also, have you checked if this changes anything for make test?

jiax1994 commented 1 year ago

Also, have you checked if this changes anything for make test?

i ran the test suite and compared the results using builds before vs after the change, the test result stays the same. A few tests seem outdated and failed before the change as well (at develop), i wonder if that's expected at start point? Here are the tests failed: minccalc-test.sh mincmorph-test run_test_progs.sh xfmconcat_01.sh xfmconcat_02.sh Most failures are due to outdated command options, directories and missing volumes

vfonov commented 1 year ago

Thank you, seems to be ok.

jiax1994 commented 1 year ago

Can you provide an example file and test showing the bug and how things act after this patch?

yes, i managed to create an example file from existing testing file under /Testing/test-rnd.mnc, here is how to create the example input file to trigger the bug mincreshape -float -colsize 125 -rowsize 1 test-rnd.mnc test-rnd-reshaped.mnc -clobber mincconcat -concat_dimension time test-rnd-reshaped.mnc test-rnd-reshaped-4d.mnc -clobber Basically the commands above convert the test-rnd.mnc to a 4D float volume with reshaped image-data test-rnd-reshaped-4d.mnc. Remember this bug happens in release mode, so make sure to test with mincresample executable built in release mode. i will also open a new PR for updating the test of mincresample