Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Round label values before voting #23

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

pipitone
Copy link
Contributor

This handles the cases where labels are floats that are only near their int label value.

@gdevenyi mentioned to me there are changes in the minc-tools pipe for handling label values better? In any case, we were recently bitten by float labels not voting correctly and this fixes that in a reasonable way (rounding seems to be what the other label-aware tools have been doing, afaik).

This handles the cases where labels are floats that are only near their int label value.
@bcdarwin
Copy link
Member

Perhaps there should also be some (by default small but settable) tolerance in case of weird rounding/conversion issues earlier in a pipe?? @cfhammill has dealt with such weird issues recently, wonder what he thinks ... also @mcvaneede

@gdevenyi
Copy link

I believe the problems @cfhammill was having was due to slice/volume normalized original label files due to processing with mincresample. I worked with him to debug the CoBrAlab original input label files to fix this and I believe at this point that's all be resolved

This change would probably fix the issue by masking the underlying cause. I do have concerns as to whether that is a good idea.

@pipitone is referring to the recent changes documented here http://www.bic.mni.mcgill.ca/pipermail/minc-development/2016-March/001237.html

@pipitone
Copy link
Contributor Author

I guess the question is more basically: are floating point label values allowable at all? If they aren't, voxel_vote (or pyminc) should probably detect that and complain. If they are, then what's the rule? It looks like to me rounding is what's normally done, in which case rint() isn't masking an issue, it's just doing what's expected.

@bcdarwin
Copy link
Member

This change would probably fix the issue by masking the underlying cause. I do have concerns as to whether that is a good idea.

That's why I suggested a small tolerance, say 0.01 by default? (Or something even smaller, essentially just performing the float->fixed conversion automatically.)

@gdevenyi
Copy link

I'd be okay with some kind of tolerance addition, but I'd rather it be optional rather than default.

I would think that floating point labels (or wrong label values) indicates issues earlier in a pipeline.

@mcvaneede
Copy link
Member

We're slowly starting to move towards having tools that produce proper label files. I agree with Gabriel that the default behaviour should produce an error when non-integer values are detected (something we don't have now), but I'd be good to also allow for rounding given a certain amount of tolerance.

This means we need to change the behaviour of some other programs as well. As Jon mentioned, there are other tools that round values of label files with as much tolerance as 0.5. For instance here:

https://github.com/Mouse-Imaging-Centre/minc-stuffs/blob/master/src/label_volumes_from_jacobians.c#L121-L123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants