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

[ENH] add a save config function #193

Merged
merged 7 commits into from
Mar 23, 2022
Merged

[ENH] add a save config function #193

merged 7 commits into from
Mar 23, 2022

Conversation

Remi-Gau
Copy link
Contributor

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 Remi-Gau changed the base branch from master to dev March 18, 2022 16:51
@Remi-Gau Remi-Gau linked an issue Mar 18, 2022 that may be closed by this pull request
@Remi-Gau
Copy link
Contributor Author

fixes #194

@codecov
Copy link

codecov bot commented Mar 18, 2022

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%) ⬆️

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%) ⬇️
src/createJson.m 84.84% <94.44%> (+19.84%) ⬆️
src/saveCfg.m 100.00% <100.00%> (ø)
src/checkCFG.m 100.00% <0.00%> (+1.14%) ⬆️

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

@Remi-Gau
Copy link
Contributor Author

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 Remi-Gau requested a review from CerenB March 18, 2022 17:15
@Remi-Gau
Copy link
Contributor Author

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

@Remi-Gau
Copy link
Contributor Author

@CerenB
Copy link
Collaborator

CerenB commented Mar 21, 2022

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
Copy link
Contributor Author

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
Copy link
Collaborator

CerenB commented Mar 21, 2022

I updated the submodule.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Mar 21, 2022

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
Copy link
Collaborator

CerenB commented Mar 22, 2022

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
Copy link
Contributor Author

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):

bf.entities.date = '';

@Remi-Gau
Copy link
Contributor Author

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

@Remi-Gau Remi-Gau merged commit 1d656b6 into cpp-lln-lab:dev Mar 23, 2022
@Remi-Gau Remi-Gau deleted the save_config branch March 23, 2022 18:13
@Remi-Gau Remi-Gau mentioned this pull request Mar 24, 2022
@CerenB
Copy link
Collaborator

CerenB commented Apr 13, 2022

Cool!
here is my update on the matter.

I do not use that bold.json from cpp-bids output 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
Copy link
Contributor Author

Cool! here is my update on the matter.

I do not use that bold.json from cpp-bids output 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!

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.

save config at run time
2 participants