-
Notifications
You must be signed in to change notification settings - Fork 162
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
Coregistration and exporting it to BIDS #647
base: master
Are you sure you want to change the base?
Conversation
@@ -85,7 +91,7 @@ | |||
%% ===== ASK PARAMETERS ===== | |||
% Ask user to set the parameters if they are not set | |||
if (nargin < 4) || isempty(erodeFactor) || isempty(nVertices) | |||
res = java_dialog('input', {'Number of vertices [integer]:', 'Erode factor [0,1,2,3]:', 'Fill holes factor [0,1,2,3]:', '<HTML>Background threshold:<BR>(guessed from MRI histogram)'}, 'Generate head surface', [], {'10000', '0', '2', num2str(sMri.Histogram.bgLevel)}); | |||
res = java_dialog('input', {'Number of vertices [integer]:', 'Erode factor [0,1,2,3]:', 'Fill holes factor [0,1,2,3]:', '<HTML>Background threshold:<BR>(guessed from MRI histogram)'}, 'Generate head surface', [], {'15000', '0', '0', num2str(sMri.Histogram.bgLevel)}); |
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.
what is the rationale for changing the default value? Unless there is a very good reason, i think we should not change the default value as it might be very confusing for most user
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.
Hi Edouard,
As I mentioned, this PR is not ready for review and some changes (this included) were not directly part of the coregistration goal.
That being said, I spent a lot of time trying to improve generating the head surface (to improve coregistration). Initially, I had a lot of MRIs with noise near the chin and the algorithm was "spending" all its vertices generating a complex beard-like surface in that area, leaving the rest of the head too smooth, in particular the ears, which made manual coregistration, or evaluating the coregistration, difficult. So the goal with this particular change (one of many) was to reduce unnecessary smoothing (more vertices, no erosion/fill) to better see the ear details.
By the way, I saw you recently had work related to distance and there might be some overlap here. I had also implemented a slower but accurate point-surface distance function, a different way to smooth surfaces (based on previous work), and modified the head point - surface fitting routine. I think all these are all in this PR, but they should be considered separately, if at all, i.e. I'll likely remove them from this PR.
Again, the immediate goal was to share the code to export BIDS coregistration. It may take quite some time for me to get this ready for a real review.
This is still a work in progress, a revised version of this previous closed PR: #511
I'm putting it here now to share it in its current state with users that also needed this functionality.
The main features are:
Note: This relatively old branch of mine contains other related changes, e.g. changes to generating the head surface and the automatic registration with head points, which would likely be removed from this PR prior to being ready for review.