-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feat: Update for ultraliser> 0.4 #18
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
=======================================
Coverage ? 79.91%
=======================================
Files ? 9
Lines ? 453
Branches ? 0
=======================================
Hits ? 362
Misses ? 91
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@arnaudon Does |
I tried on cerebellum, and it looked reasonable with this setup. If I were to decide, I would only use a simple marching cube, I'm not sure about the added value to have sub-voxel resolution here, but I don't know the rest of the algo enough. Are there more test one could do to validate this version? It does not seem tested here. Maybe @asoplata could also try it on his atlas to see? I will keep working on cerebellum atlas in the coming days to better validate, we can keep this open for now, np! |
Well, that's already a big sign.
Unfortunately I'm not aware of any, so I'm not sure what we can do.
Ok, that's fine with me. I'm also fine with merging it if you need it. |
this will be breaking change, not compat with <4 (at least not with 0.1.0 that was used before). So maybe I can make sure it still works with both? I'm not sure how this will impact the atlas pipeline, etc... |
There is no validation for placement hints (AFAIK) so the only way to test the impact on the Atlas pipeline is to try it :)
with
|
If we make a new major release, would it be ok? I'm not sure I can make this work with all the nexus things underneath |
I believe I only tested the thalamus placement-hints using the newer ultraliser This also should not affect the current Atlas pipeline, since as Leo said, it only runs the isocortex, and to my knowledge, the isocortex placement-hints algorithm doesn't use ultraliser at all (it uses the "voxel-based" instead of "mesh-based" method). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No impact on the current Atlas pipeline:
#18 (comment)
This works with ultraliser==0.4.1, which will appear on spack shortly BlueBrain/spack@567c676