search

Mouse-Imaging-Centre / pyminc

A python interface to the MINC 2 library, allowing use of numpy arrays to access MINC data, and other such similar goodies, developed by Jason Lerch
http://en.wikibooks.org/wiki/MINC/Tutorials
Other
14 stars 8 forks source link

Fix inverted logic in label code #31

Closed gdevenyi closed 4 years ago

gdevenyi commented 5 years ago

Fixing inverted logic in label code which accidentally allows label files to have scaling applied to them.

Tested with:

from pyminc.volumes.factory import *
infile = volumeFromFile("/home/gdevenyi/localtemp/brain1_labels.mnc", dtype='ubyte', labels=True)
outfile = volumeFromInstance(infile, "/tmp/test.mnc", dtype='ubyte', volumeType='ubyte', labels=True)

outfile.data = infile.data

outfile.writeFile()
outfile.closeVolume()

Loads in a label file and writes it out.

Before(note voxel value vs real value in bottom left) image

After(voxel and real value now correspond) image

gdevenyi commented 5 years ago

Followup here.

gdevenyi commented 4 years ago

Another followup

gdevenyi commented 4 years ago

@bcdarwin is there any interest in this?

bcdarwin commented 4 years ago

Yep, looking into a couple Pyminc issues at the moment, hope to look at this tonight

bcdarwin commented 2 years ago

@gdevenyi -- looking at this again, I now believe this is incorrect. For instance, if I use Display to open a label file created after this change, Display believes the file contains very large values and thus scales the intensity of the labels into oblivion. Similarly mincstats produces a warning that the maximum value is incorrect.

My understanding is that MINC implements a global scaling (controlled by the volume-wide voxel_range and voxel_valid_range) even if slice scaling is disabled (but I suspect that most label-related tools actually ignore this in favour of the voxel values?). However I'm not sure of the preferable way to deal with potential rounding issues associated with the label volume case. I believe that either this change should be reverted or these two lines should be deleted for the label case.

gdevenyi commented 2 years ago

I did this because it fixed something that was failing for me. I think we need to produce a set of test cases and make sure that voxel_vote and pyminc work properly on them to solve all these corner cases.

bcdarwin commented 2 years ago

I agree that more tests are needed for sure -- I've set up CI for Pyminc recently but it's clearly missing the relevant test, and voxel_vote doesn't have any automated test suite right now. It's been a while, but do you by chance emember what tool wasn't working?

gdevenyi commented 2 years ago

The test code/images in the first post, loading and saving a label file modified the global scaling before, and not after.

i.e. For labels, the real and voxel values should be the same for label files.

bcdarwin commented 2 years ago

After speaking with @jgsled I am convinced (unless I misunderstood the names of the API functions in question) that setting voxel_valid_range to voxel_range is the correct way to eliminate the scaling. Using Pydpiper 0.52, I reproduce the discrepancy between real/voxel values reported by Display above, but note that e.g. the DSURQE atlas itself [admittedly, creating using Pyminc] has the same issue (the Dorr 2008 atlas does not since it has 250 labels and its valid range has been set to 255 which corresponds with the max ushort). Thus, I strongly believe this commit should be reverted, but am unsure if there's any remaining issue.

jgsled commented 2 years ago

Hi Ben,

That’s correct. MINC allows for voxel intensity scaling to be either global or per slice, but never absent.

cheers,

John

From: Ben Darwin @.> Reply-To: Mouse-Imaging-Centre/pyminc @.> Date: Thursday, April 28, 2022 at 10:55 AM To: Mouse-Imaging-Centre/pyminc @.> Cc: Subscribed @.> Subject: Re: [Mouse-Imaging-Centre/pyminc] Fix inverted logic in label code (#31)

@gdevenyihttps://github.com/gdevenyi -- looking at this again, I now believe this is incorrect. For instance, if I use Display to open a label file created after this change, Display believes the file contains very large values and thus scales the intensity of the labels into oblivion. Similarly mincstats produces a warning that the maximum value is incorrect.

My understanding is that MINC implements a global scaling (controlled by the volume-wide voxel_range and voxel_valid_range) even if slice scaling is disabled. However I'm not sure of the preferable way to deal with potential rounding issues associated with the label volume case. I believe that either this change should be reverted or these two lines should be deleted for the label case.

— Reply to this email directly, view it on GitHubhttps://github.com/Mouse-Imaging-Centre/pyminc/pull/31#issuecomment-1112309108, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAO7TDE46KOODAAQS6ZKK23VHKRGHANCNFSM4IPHZTQA. You are receiving this because you are subscribed to this thread.Message ID: @.***>