-
Notifications
You must be signed in to change notification settings - Fork 237
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
Improve label sampler efficiency #6
Comments
I see the warning from the doc, but did not look at the code yet, Actually I was looking for a sampler, that contains the center of the patch within the label. (it is a more conservative alternative to the current setting "at least one non background voxel") The implementation just need to randomly sample (for the patch center) the coordinate of the label mask ... it should be fast ? |
Yes I think that's the way to go. That's how NiftyNet works as well. I'll add a more generic weighted sampler. Then a label map can be used as sampler. I need to think how to handle sampler images, though. That's why I also added a |
ok |
Hi @fepegar, we will probably use the I thought we could use the same signature with an extra argument that would be a list of probabilities for the different values in the label with default as For the implementation itself, we could just draw a random number that decide which label value to focus on, then choose randomly a corresponding voxel from the label and return the patch centered on this voxel (with padding if needed). Would that be enough or am I missing a deeper design choice that would lead to a different solution? In any case I would be happy to provide a PR if needed. |
I'll take a look at this next week. Before I forget, this is related to #50. I think we can follow NiftyNet's approach, i.e. using weighted samplers. Then a label sampler would be a specific case, where the label map can be used as a weight map. They can be tricky, this needs to be done carefully (NifTK/NiftyNet#432). |
Hi @fepegar! Any update on this issue? |
Sorry, I've been busy. I'll hopefully look into it on Friday. |
No problem! |
I just spent some hours on this. See #175. |
Let me know if that implementation would work for you and I'll merge the changes ASAP. |
No description provided.
The text was updated successfully, but these errors were encountered: