LCSR-CIIS / ambf_util_slicer_plugin

some utilities for quickly converting volumes, markups from 3DSlicer into format accepted by AMBF simulator
BSD 2-Clause "Simplified" License
3 stars 3 forks source link

Error: FileNotFoundError and Data Size does not match dimensions #10

Closed adnanmunawar closed 1 year ago

adnanmunawar commented 1 year ago

Hi Henry,

I think there may be some confusion in the workflow of saving the ADF file. I completed all the steps as I previously did but somehow ended up with these errors.

  1. FileNotFoundError
  2. Data Size does not match dimensions

This is output in the Python console.

Screenshot from 2023-05-19 12-01-54

This is what my configuration looks like:

Screenshot from 2023-05-19 12-02-11

I checked and I do have the directory ~/Documents/adf_volume.

Another thing to note is that even though I only clicked Export LabelMap to PNGs for AMBF, the Python console shows the method onRefreshYamlButton was called. Is this the intended behavior? If so, would it not cause issues if a file does not already exist?

adnanmunawar commented 1 year ago

Curious incurious, I tested with ~/Documents folder instead of ~/Documents/adf_volume and I don't see the same error. Hmmm!!!

Nevermind, that is because I already had a volume.yaml file in there, so the issue is the same.

htp2 commented 1 year ago

Hmmm. This will be fun to chase down... Probably some change that crept in with the last fix.

Not sure when I can get to it, should be fairly easy to read through the Python code if you have bandwidth. Like you said it really looks like an issue with the workflow order.

adnanmunawar commented 1 year ago

Found the culprit. This line (https://github.com/htp2/ambf_util_slicer_plugin/blob/master/AMBF_utils.py#L506). The data_size and dimensions are not the same, and I see that the x and y are flipped between the two. Is this due to LPS vs RAS or just the way Slicer stores arrays?

htp2 commented 1 year ago

Ah, my unofficial test volume was probably "square" in x and y so this wasn't picked up with the last change...

The switch is explained in the block of text above: line ~493. Basically, the 2D slice pixel origin and width/height vs height/width definitions are different, so to get things to load correctly, the matrix gets rearranged a bit including "x" and "y" flipping. This is where using i,j,k instead of x,y,z for voxel indexes / dimensions would have been much better!

It's a bit confusing , but I think just for the size check dimensions[0] and dimensions[1] should flip. However, realistically, the check might not even be needed and currently is giving a false positive.

Either that or there's actually a miss here and we have to change the logic a bit for these i =/= j cases. If you comment out the test for now, does everything work?

adnanmunawar commented 1 year ago

Yes that makes sense. I kept the check and just removed the return statement to let the method run till the end and save the ADF file and then it seems to work. Please see the PR above.

htp2 commented 1 year ago

11 fixed the issue, but I'm going to leave this open as we still need to fix the warning message as I believe it is checking the wrong thing and will give a confusing output.

htp2 commented 1 year ago

Fixed in #12