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

vertex cleaning, especially for DG #300

Closed
wants to merge 8 commits into from
Closed

Conversation

jordandekraker
Copy link
Collaborator

Here I tried to clean up some misplaced vertices, which occurs especially often in DG. to do this, I employ s strategies:

  1. in addition to filling NaN vertices, we also find bad vertices, which are defined by their distance to a smoothed surface
  2. create_warp.py now uses a 256,128,16 laplace coord grid, instead of 0-1x0-1x0-1. This should make the determination of neighbours a bit cleaner

NOTE I also played around with some config options. I'm not sure whether this helps, so i'm going to undo the next changes in my next commit. We should test on more cases to find out.

@jordandekraker jordandekraker added the breaking New feature that breaks comptability with previous versions label Jun 12, 2024
@akhanf
Copy link
Member

akhanf commented Jun 13, 2024

With PRs like this that impact all workflows (ie not adding a new feature, but changing how a existing computation is done), it would be good to set-up some regression testing (wet-run, not just dry-run) to assess how much an impact these changes make.

Would be good to do this for a variety of datasets too. Would have to decide on a few things (what the tests will actually evaluate, what datasets, how to perform the tests efficiently on github actions), so I think a separate issue needs to be spawned.

@jordandekraker
Copy link
Collaborator Author

agreed. I'm doing some local testing, but it would be good to have something more systematic.

Similarly, i'm adding a bigbrain template (at 40um which is what we did in the BigBrain neuroimage paper) to the OSF repo so i can see how dentate processing works there. Will PR it into this branch shortly (or maybe not so shortly, it takes a long time to get the Laplace solutions at that resolution!). We could add that as a possible wet-run as well, but its a bit niche and heavy so maybe i'll stick to just testing it locally.

@jordandekraker
Copy link
Collaborator Author

closed with #306

@jordandekraker jordandekraker deleted the vertexcleaners branch June 26, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking New feature that breaks comptability with previous versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants