BIDS features - MATLAB 2022 internship #484

mathieuboudreau · 2023-03-09T20:04:47Z

Opening this PR for Agah while he's away on intership in Stanford - I'm going to do a review of the current changes and what appears to be left to do.

- A json file describing input/output data and protocol units with reference to the example implementation and example dataset. - When a new model added, this file must be updated. - To be added to the documentation

- Show unit in GUI

- Move scaling functions from Abstract model to outside - Wrap Prot scaling around the FitData and ParFitData

- BIDS specification mappings for anat, dwi, fmap - BIDS to qMRLab input rules - qMRLab to BIDS output rules - qMRLab model registry - Unit specifications - README explaining the convention

- Add unit selection - Improve parallel fit options - Add README

- Directory navigation was needed.

- Add model specific notes and refs inc BIDS - Refactor BinderHub badge insert - Update references

- Add unit I/O management for all models - Set checkAnterior version 2,5,0 to account for OriginalProtEnabled and setUserProtUnits - Edit panel texts to avoid unit description duplication, wherever applicable.

- FITBIDS uses all /dev configs in relation to the new model registry to map a BIDS formatted data into a qMRLab process. Emits env var ISBIDS. - getUserPreferences reads /usr configs and determines whether choices will be overridden for BIDS (env var ISBIDS) - modelRegistry infers the mapping rules based on set preferences - Add helper functions

- To store app data regarding unit scaling

- Add gitignore rule not to exclude - Save outputs in BIDS formats and units

- Scale input/outputs maps as necessary without affecting how the original model accepts input data and

- Use helper functions - Add scaling function

- Improve documentation template - Make static model to read list from model registry - Output display units

- Accounted by the new model registry

- Fix misc issues

codecov · 2023-03-09T20:20:54Z

Codecov Report

Merging #484 (6d8646e) into master (036ff20) will decrease coverage by 0.91%.
The diff coverage is 46.57%.

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

@@            Coverage Diff             @@
##           master     #484      +/-   ##
==========================================
- Coverage   47.40%   46.50%   -0.91%     
==========================================
  Files         260      272      +12     
  Lines        8648     9631     +983     
==========================================
+ Hits         4100     4479     +379     
- Misses       4548     5152     +604

Flag	Coverage Δ
matlab	`46.50% <46.57%> (-0.12%)`	⬇️
octave	`?`

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

Impacted Files	Coverage Δ
src/Common/AbstractModel.m	`71.25% <ø> (+5.00%)`	⬆️
src/Common/FitBIDS.m	`0.00% <0.00%> (ø)`
src/Common/FitResultsSave_BIDS.m	`0.00% <0.00%> (ø)`
src/Common/qMRLab_static_Models.m	`0.00% <0.00%> (ø)`
src/Common/saveUserPreferences.m	`0.00% <0.00%> (ø)`
src/Common/tools/versionChecker.m	`0.00% <0.00%> (-62.17%)`	⬇️
src/Common/units/validatePanelUnits.m	`0.00% <0.00%> (ø)`
...els_Functions/MP2RAGE/func/T1B1correctpackageTFL.m	`84.74% <ø> (ø)`
src/Common/getUserPreferences.m	`32.50% <32.50%> (ø)`
src/Models/T1_relaxometry/vfa_t1.m	`41.00% <42.85%> (-34.45%)`	⬇️
... and 27 more

... and 17 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

mathieuboudreau · 2023-03-09T20:21:03Z

Tagging @vijayiyer05 from the MATLAB team!

mathieuboudreau · 2023-03-09T20:22:38Z

Deploy/Docker/MINIMAL/Dockerfile

@@ -28,7 +28,7 @@ RUN echo ${TAG}; \
    mkdir $HOME/work; \
    cd $HOME/work; \    
    git clone --recurse-submodules -b ${TAG} --single-branch --depth 1 https://github.com/qMRLab/qMRLab.git;\
-    chmod -R 777 $HOME/work/qMRLab; \
+    chmod -R 777 cd $HOME/work/qMRLab; \


This cd seems out of place within a chmod command, I'm making a note to manually test the Dockerfile in case this needs to be fixed.

I noticed this one too, but I did not want to push changes for it not to break a book elsewhere that maybe using the latest tag. Thanks!

mathieuboudreau · 2023-03-09T20:23:41Z

Deploy/Documentation/Notes/docModelNotes.json

@@ -24,6 +24,12 @@
          "%   |-------------- sub-01_TB1map.nii.gz",
          "%   |-------------- sub-01_TB1map.json",
          "% ",
+          "% <https://osf.io/rt642/download Download> our example |TB1DAM| BIDS dataset and process:",


BIDS demo documentation, making a note to myself to test this manually.

Seems like it gets repeated for each model, so also making a note to check that this automated approach at generating the BIDS documentation is general enough for all models that it's generated for.

The notes field is model specific to add BIDS-specific content and citation strings to the reference articles:

Handled by the following

qMRLab/src/Common/qMRgenBatch.m

Lines 196 to 221 in af5c37f

% Generate model specific commands ====================== END

for ii =1:length(notesJson.notes)

if strcmp(Model.ModelName,notesJson.notes{ii}.model)

if isfield(notesJson.notes{ii},'note')

noteTexts.notes = cellstr(notesJson.notes{ii}.note');

else

noteTexts.notes = noNotes;

end

if isfield(notesJson.notes{ii},'citation')

noteTexts.citation =cellstr(['% ' notesJson.notes{ii}.citation]);

else

noteTexts.citation = noCitation;

end

break;

else

noteTexts.notes = noNotes;

noteTexts.citation = noCitation;

end

end

% Replace jokers ====================== START

mathieuboudreau · 2023-03-09T20:26:40Z

dev/BIDS_to_qmrlab_input_mappings.json

@@ -0,0 +1,112 @@
+{


Note: qMRLab BIDS files currently in a temporary dev/ directory, which will need to be renamed (and possible moved) prior to merging.

The dev/ directory was not meant to be temporary, but we can rename it to config if you think that it makes more sense.

mathieuboudreau · 2023-03-09T20:30:01Z

dev/README_FIRST.md

@@ -0,0 +1,226 @@
+# ⚠️ Warning


BIDS documentation for the developers. Need to check if this should be moved to our wiki and/or ReadTheDocs before the PR is merged.

I think this should be ho into the wiki page, we better keep RTD cleaner for users.

mathieuboudreau · 2023-03-09T20:32:42Z

The test suite is failing, but doesn't appear to be related to the BIDS feature (it's during the MTsat batch example run). Note to self to check/resolve this, and see if some non-BIDS related code was touched in this branch that would have also raised this error.

mathieuboudreau · 2023-03-09T20:34:10Z

Code coverage is failing also, likely due to the BIDS code being in dev/ and not src/ (where code is tested). Will need to move dev somewhere inside src, and see if the code is tested then to reach our code coverage threshold.

mathieuboudreau · 2023-03-14T00:51:47Z

src/Models/Magnetization_transfer/mt_sat.m

@@ -107,34 +104,39 @@

            B1params = obj.options.B1correctionfactor;

-            [FitResult.MTSAT, R1] = MTSAT_exec(data, MTparams, PDparams, T1params, B1params);
-            FitResult.T1 = 1./R1;
+            [MTSAT, R1, R1cor, MTsatcor] = MTSAT_exec(data, MTparams, PDparams, T1params, B1params);


The test suite currently fails due to the change on this line. Two extra outputs were set (R1cor and MTsatcor), but the function that's being called never defined these extra outputs and still only outputs 2 values:

qMRLab/src/Models_Functions/MTSATfun/MTSAT_exec.m

Line 3 in 21bee21

function [MTsat, R1] = MTSAT_exec(data, MTParams, PDParams, T1Params, B1Params)

Despite an attempt at a workaround below on lines 119 and 123, the ~isempty call never get set since the code errors here.

It's not clear to me what exactly the two extra variables are expected to be (I could imagine that MTsatcor would maybe be with B1 correction from the code, but not sure about R1cor), I'll instead just add some placeholder "empty" variables that get outputted, and we can touch on this during the full review later on.

Handled in dc2f259, but will need to revisit and/or check with Agah later what the intended behaviour was while this was being developped.

I remember Juan was working on this, the expected variables should be un-corrected (B1) maps. Most likely defaults were not matching what the test was expecting with some variable matching. Thanks for the fix, a more thorough solution to follow.

agahkarakuzu and others added 30 commits March 2, 2021 14:21

Switch default method to json

bed975d

Test and all

4289962

ADD nativeModelUnits.json

8cd22ad

- A json file describing input/output data and protocol units with reference to the example implementation and example dataset. - When a new model added, this file must be updated. - To be added to the documentation

OutputMap Unit defs

ae1691f

MP2RAGE B1map scaling factor 1

3da6567

xnames, map, units and unit class relationships

5bda9e2

Add explanations for remaining xnames

14b04ea

Minor fixes

f61ef2d

Rename and add more

7c32f01

Finalize BIDS, unit and usr mappings

defca67

Unit checks for inputs

1e84574

- Show unit in GUI

Improve functionality

d7b8321

Enable output scaling

87e784a

Make dev's life easier

bd22f11

Octave comp

f5280ad

Enable input standardization

dfeab5f

Typo

97bf94a

Implement protocol param unificaiton

2cb7be0

Remove overhead

2cbf8d5

Add some more units

c4ada1a

Refactor functions

2fd0411

- Move scaling functions from Abstract model to outside - Wrap Prot scaling around the FitData and ParFitData

GUI unit validation

a16fa93

Add comments

a95c5df

Various fixes

85b9b3e

Add ProtScale in all constructors

438721e

Comment

9e14430

Gitignored :D

e7cddfe

Add Dcsf of Charmed to registry

5b402f6

Patch models and drop units from tabletips

0796e30

Add Dintra to the mappings list

38f9ec9

agahkarakuzu and others added 16 commits October 11, 2022 13:24

[ENH] Add /dev configuration folder

a60dd69

- BIDS specification mappings for anat, dwi, fmap - BIDS to qMRLab input rules - qMRLab to BIDS output rules - qMRLab model registry - Unit specifications - README explaining the convention

[ENH] User configurations

4ccafd0

- Add unit selection - Improve parallel fit options - Add README

[FIX] Dockerfile chmod directive

0fa7e36

- Directory navigation was needed.

[ENH] Major - Documentation

4aa682f

- Add model specific notes and refs inc BIDS - Refactor BinderHub badge insert - Update references

[ENH] Major - Update models

bc995e3

- Add unit I/O management for all models - Set checkAnterior version 2,5,0 to account for OriginalProtEnabled and setUserProtUnits - Edit panel texts to avoid unit description duplication, wherever applicable.

[ENH] Add EnvDetails hidden property

e1cc2b9

- To store app data regarding unit scaling

[ENH] FitResultsSave BIDS

d870d15

- Add gitignore rule not to exclude - Save outputs in BIDS formats and units

[ENH] Major - Adjust FitData for unit management

8f39fd7

- Scale input/outputs maps as necessary without affecting how the original model accepts input data and

[ENH] Adapt ParFitData for units

5c7a13e

- Use helper functions - Add scaling function

[ENH] Adapt common functions for BIDS and units

f743978

- Improve documentation template - Make static model to read list from model registry - Output display units

[ENH] Helper function for updating unit labels

d33a8b6

[ENH] Add helper functions for unit management

89ddb37

[FIX] Remove default method file

afd4f9f

- Accounted by the new model registry

[BUG] Version manager

fbfefab

- Fix misc issues

Merge branch 'master' into matlabsummer22

21bee21

mathieuboudreau commented Mar 9, 2023

View reviewed changes

mathieuboudreau commented Mar 14, 2023

View reviewed changes

mathieuboudreau and others added 5 commits March 13, 2023 21:52

Bug hotfix: missing output variables

dc2f259

retrigger checks

2c973f3

retrigger checks

e691533

Merge branch 'usrjson' into matlabsummer22

07ab01d

[ENH] Merge latest from dev branch

af5c37f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BIDS features - MATLAB 2022 internship #484

BIDS features - MATLAB 2022 internship #484

mathieuboudreau commented Mar 9, 2023

codecov bot commented Mar 9, 2023 •

edited

mathieuboudreau commented Mar 9, 2023

mathieuboudreau Mar 9, 2023

agahkarakuzu May 5, 2023

mathieuboudreau Mar 9, 2023

mathieuboudreau Mar 9, 2023

agahkarakuzu May 5, 2023

mathieuboudreau Mar 9, 2023

agahkarakuzu May 5, 2023

mathieuboudreau Mar 9, 2023

agahkarakuzu May 5, 2023

mathieuboudreau commented Mar 9, 2023

mathieuboudreau commented Mar 9, 2023

mathieuboudreau Mar 14, 2023

mathieuboudreau Mar 14, 2023

agahkarakuzu May 5, 2023

	% Generate model specific commands ====================== END


	for ii =1:length(notesJson.notes)

	if strcmp(Model.ModelName,notesJson.notes{ii}.model)
	if isfield(notesJson.notes{ii},'note')
	noteTexts.notes = cellstr(notesJson.notes{ii}.note');
	else
	noteTexts.notes = noNotes;
	end

	if isfield(notesJson.notes{ii},'citation')
	noteTexts.citation =cellstr(['% ' notesJson.notes{ii}.citation]);
	else
	noteTexts.citation = noCitation;
	end

	break;
	else
	noteTexts.notes = noNotes;
	noteTexts.citation = noCitation;
	end

	end
	% Replace jokers ====================== START

BIDS features - MATLAB 2022 internship #484

Are you sure you want to change the base?

BIDS features - MATLAB 2022 internship #484

Conversation

mathieuboudreau commented Mar 9, 2023

codecov bot commented Mar 9, 2023 • edited

Codecov Report

mathieuboudreau commented Mar 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathieuboudreau commented Mar 9, 2023

mathieuboudreau commented Mar 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 9, 2023 •

edited