Remi-Gau commented 2 years ago

Because createJson now only saves a super lightweight version in bold.json, we have no way of tracking what was used for an experiment which will be annoying for reproducibility.

This function should take over and allow some flexibility in terms of output.

Remi-Gau commented 2 years ago

fixes #194

codecov[bot] commented 2 years ago

Codecov Report

Merging #193 (3c20a7d) into dev (ab06459) will increase coverage by 1.66%. The diff coverage is 92.85%.

@@            Coverage Diff             @@
##              dev     #193      +/-   ##
==========================================
+ Coverage   79.93%   81.60%   +1.66%     
==========================================
  Files          29       30       +1     
  Lines         658      674      +16     
==========================================
+ Hits          526      550      +24     
+ Misses        132      124       -8

Flag	Coverage Δ
unittests	`81.60% <92.85%> (+1.66%)`	:arrow_up:

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

Impacted Files	Coverage Δ
src/utils/getFullFilename.m	`50.00% <50.00%> (-50.00%)`	:arrow_down:
src/createJson.m	`84.84% <94.44%> (+19.84%)`	:arrow_up:
src/saveCfg.m	`100.00% <100.00%> (ø)`
src/checkCFG.m	`100.00% <0.00%> (+1.14%)`	:arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Remi-Gau commented 2 years ago

3 use cases:

The most common: enough info in cfg to create a filename that ends in _cfg.json in the subject directory and matches the run fielname.

cfg = setUp();
filename = saveCfg(cfg)

filename =
    '/home/remi/github/CPP_BIDS/src/../output/source/sub-001/ses-001/func/sub-001_ses-001_task-testTask_run-001_date-202203181811_cfg.json'

Not enough info in cfg to create a filename

cfg.testingDevice = 'mri';
filename = saveCfg(cfg)

filename =
    '/home/remi/github/CPP_BIDS/date-202203181809_cfg.json'

filename is provided

cfg.testingDevice = 'mri';
filename = saveCfg(cfg, fullfile(pwd, 'cfg', 'cfg.json'))
filename =
    '/home/remi/github/CPP_BIDS/cfg/cfg.json'

Remi-Gau commented 2 years ago

Once this is merged, this will be incoporated into the different localizers and experiments.

Remi-Gau commented 2 years ago

Rendered doc: https://cpp-bids--193.org.readthedocs.build/en/193/function_description.html#src.saveCfg

CerenB commented 2 years ago

As usual, I might have done sth wrong but what I got is a change in _event.json and _event.tsv as well. sub-066_ses-001_task-mototopy_run-001_date-202203211705_events.tsv sub-066_ses-001_task-mototopy_run-001_date-202203211705_events.json

In this case, it might be annoying also to change cpp-bids function which removes the suffix.

I used very similar to what I was using this function, so maybe I did use the new function wrong - at the end of the main script of a fmri experiment:

getResponse('stop', cfg.keyboard.responseBox);
    getResponse('release', cfg.keyboard.responseBox);

    % create json for bold
    createJson(cfg, 'func');

    % save config info
    saveCfg(cfg);

    farewellScreen(cfg);
    cleanUp();

catch

    % save config info
    saveCfg(cfg);

    cleanUp();
    psychrethrow(psychlasterror);

end

Remi-Gau commented 2 years ago

Just to know, did you just grab that function and added it into your experiment code or did you update the whole CPP_BIDS submodule?

CerenB commented 2 years ago

I updated the submodule.

Remi-Gau commented 2 years ago

As usual, I might have done sth wrong but what I got is a change in _event.json and _event.tsv as well. sub-066_ses-001_task-mototopy_run-001_date-202203211705_events.tsv sub-066_ses-001_task-mototopy_run-001_date-202203211705_events.json

Are you saying that before you used to have filenames like:

sub-066_ses-001_task-mototopy_run-001_events_date-202203211705.tsv

and now you get this?

sub-066_ses-001_task-mototopy_run-001_date-202203211705_events.tsv

CerenB commented 2 years ago

yes exactly. I didn't have time to do a proper check yet. I'll see where the issue stems from this afternoon and write back!

Remi-Gau commented 2 years ago

Don't look further: this is normal.

This was changed back there: https://github.com/cpp-lln-lab/CPP_BIDS/pull/186/files?file-filters%5B%5D=.m&show-deleted-files=true&show-viewed-files=true#r832040816

Just added a comment to that old PR to help you track it.

The new version convertSourceToRaw now removes this date this way (by realying on BIDS matlab function to create and rename BIDS files):

https://github.com/cpp-lln-lab/CPP_BIDS/blob/ab06459710a19389183367cd5220d35bc521ea0f/src/convertSourceToRaw.m#L47

Remi-Gau commented 2 years ago

Will merge this as we sort of need it for our next testing session (not essential but I would prefer to have it)

CerenB commented 2 years ago

Cool! here is my update on the matter.

I do not use that bold.json from cpp-bidsoutput anymore. What do I mean by "use"?

I updated cpp-bids submodule in my exp presentation scripts, so there’s no extra column in the.tsv files produced for onsets. (side information)
I’m saving config.json output from cpp-bids . But that is for me to save stuff in source folder. All the config files are saved! I also save bold.json currently, but I do not use those anymore in any analysis.
For analysis (raw and derivatives) I am using heudiconv converter’s bold.json files. So it does extract the sliceOrder information, therefore in my scripts e.g. setParameters I do not need that sliceOrder info to be saved onto cpp-bids bold.json file. And heudiconv generated bold.json files are passing bids-validator, cpp-spm, and MRIQC.

Remi-Gau commented 2 years ago

Cool! here is my update on the matter.

I do not use that bold.json from cpp-bidsoutput anymore. What do I mean by "use"?

* I updated `cpp-bids` submodule in my exp presentation scripts, so there’s no extra column in the` .tsv` files produced for onsets. (side information)

* I’m saving `config.json` output from `cpp-bids` . But that is for me to save stuff in `source` folder. All the config files are saved! I also save `bold.json` currently, but I do not use those anymore in any analysis.

* For analysis (`raw` and `derivatives`) I am using [heudiconv](https://www.youtube.com/watch?v=O1kZAuR7E00&ab_channel=JamesKent) converter’s `bold.json` files. So it does extract the `sliceOrder` information, therefore in my scripts e.g. `setParameters` I do not need that `sliceOrder` info to be saved onto `cpp-bids` `bold.json` file.
  And `heudiconv` generated `bold.json` files are passing `bids-validator`, `cpp-spm`, and `MRIQC`.

And they all lived happily ever after!

cpp-lln-lab / CPP_BIDS

[ENH] add a save config function #193

Codecov Report