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

Masking in FitData #475

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Masking in FitData #475

wants to merge 3 commits into from

Conversation

jvelazquez-reyes
Copy link
Collaborator

@jvelazquez-reyes jvelazquez-reyes commented Jul 4, 2022

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)
  • Does the qMRLab GUI open?
  • Can you change models?
  • Can you load a data folder for a model?
  • Can you view data?
  • Can you zoom in the image?
  • Can you pan out of the image?
  • Can you view the histogram of the data?
  • Can you change the color map?
  • Can you fit dataset (Fit data)?
  • Can you save/load the results?
  • Can you open the options panel?
  • Can you change option parameters?
  • Can you save/load option paramters?
  • Can you select a voxel?
  • Can you fit the data of that voxel ("View data fit")?
  • Can you simulate and fit a voxel ("Single Voxel Curve")?
  • Can you run a Sensitivity Analysis?
  • Can you simulate a Multi Voxel Distribution?
Specifications
  • Version:
  • Platform:
  • Subsystem:

@codecov
Copy link

codecov bot commented Jul 4, 2022

Codecov Report

Merging #475 (3960998) into master (8b8fb39) will decrease coverage by 5.31%.
The diff coverage is n/a.

❗ Current head 3960998 differs from pull request most recent head d803d21. Consider uploading reports for the commit d803d21 to get more accurate results

Impacted file tree graph

@@            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     
Flag Coverage Δ
matlab 42.08% <ø> (-4.54%) ⬇️
octave ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Models/Magnetization_transfer/mt_ratio.m 100.00% <ø> (ø)
src/Models/T1_relaxometry/mp2rage.m 72.72% <ø> (+4.67%) ⬆️
src/Common/sim/computeR1.m 0.00% <0.00%> (-100.00%) ⬇️
..._Functions/MTVfun/mtv_correct_receive_profile_v2.m 0.00% <0.00%> (-100.00%) ⬇️
...rc/Models_Functions/bSSFPfun/functions/bSSFP_fit.m 0.00% <0.00%> (-96.43%) ⬇️
src/Models_Functions/Filter/polyfit_3D.m 0.00% <0.00%> (-94.92%) ⬇️
src/Models_Functions/MTVfun/ordfilt3D.m 0.00% <0.00%> (-92.86%) ⬇️
.../Models_Functions/SIRFSEfun/functions/SIRFSE_fit.m 0.00% <0.00%> (-89.29%) ⬇️
src/Models/T1_relaxometry/mtv.m 11.76% <0.00%> (-88.24%) ⬇️
src/Models_Functions/MTVfun/mtv_fit3dsplinemodel.m 0.00% <0.00%> (-80.00%) ⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b8fb39...d803d21. Read the comment docs.

@@ -209,9 +209,9 @@

[T1corrected, MP2RAGEcorr] = T1B1correctpackageTFL(data.B1map,MP2RAGEimg,[],MP2RAGE,[],invEFF);

FitResult.T1 = T1corrected.img;
Copy link
Member

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

Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants