-
Notifications
You must be signed in to change notification settings - Fork 58
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
Masking in FitData #475
base: master
Are you sure you want to change the base?
Masking in FitData #475
Conversation
Codecov Report
@@ Coverage Diff @@
## master #475 +/- ##
==========================================
- Coverage 47.40% 42.08% -5.32%
==========================================
Files 260 260
Lines 8647 8637 -10
==========================================
- Hits 4099 3635 -464
- Misses 4548 5002 +454
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
src/Models/T1_relaxometry/mp2rage.m
Outdated
@@ -209,9 +209,9 @@ | |||
|
|||
[T1corrected, MP2RAGEcorr] = T1B1correctpackageTFL(data.B1map,MP2RAGEimg,[],MP2RAGE,[],invEFF); | |||
|
|||
FitResult.T1 = T1corrected.img; |
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 did these names have to do with masking? I think changing the names might break existing pipeline because it's a change to the API
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.
Nothing :) I wanted to follow the naming convention we adopted for exporting B1 corrected maps. I just recalled this is already implemented in this branch.
For clarity, let me revert that FitResult.T1cor
-> FitResult.T1
, as these changes will eventually be implemented.
Purpose
Carry out the data masking process internally. Issue: #312
Approach
The masking process is carried out by the
FitData
function, instead of on the model functions.Voxelwise=1
case:https://github.com/qMRLab/qMRLab/blob/master/src/Common/FitData.m#L96-L112
Voxelwise=0
case:https://github.com/qMRLab/qMRLab/blob/master/src/Common/FitData.m#L253-L284
Open Questions and Pre-Merge TODOs
Use github checklists. When solved, check the box and explain the answer.
Review that changed source files/lines are related to the pull request/issue
If any files/commits were accidentally included, cherry-pick them into another branch.
Review that changed source files/lines were not accidentally deleted
Fix appropriately if so.
Test new features or bug fix
If not implemented/resolved adequately, solve it or inform the developer by requesting changes in your review.
Preferably, set breakpoints in the locations that the code was changed and follow allong line by line to see if the code behaves as intended.
Manual GUI tests (general)
Specifications