Skip to content

Commit

Permalink
[FIX] small fixes in createDefaultStatsModel.m, getAcquisitionTime.m …
Browse files Browse the repository at this point in the history
…& bidsResults.m (#1248)

* changed condition to check compatibility of sliceOrder and acquisitionTime

* fixed conditional statement defining bm.Nodes.GroupBy

* Update function bidsResultsSubject: identification of run level contrasts

* fixed spelling

* update citation.cff

* update citation.cff

* add test getAcquisitionTime

* adapt test data for bids stats model

* extract regex

* add tests

* revert

---------

Co-authored-by: Remi Gau <remi_gau@hotmail.com>
  • Loading branch information
d-ni374 and Remi-Gau authored Jul 1, 2024
1 parent e401f5f commit bf2e35b
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 23 deletions.
4 changes: 4 additions & 0 deletions CITATION.cff
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ authors:
affiliation: "University of Gent"
orcid: "https://orcid.org/0000-0001-8279-6118"

- family-names: Huber
given-names: Daniel
affiliation: University of Innsbruck

- family-names: "Van Audenhaege"
given-names: "Alice"
affiliation: "Université catholique de Louvain"
Expand Down
2 changes: 1 addition & 1 deletion src/bids_model/createDefaultStatsModel.m
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@

if strcmp(levelsToUpdate{i}, 'subject')
bm.Nodes{idx}.GroupBy = {'subject', 'contrast'};
elseif strcmp(levelsToUpdate{i}, 'subject')
elseif strcmp(levelsToUpdate{i}, 'dataset')
bm.Nodes{idx}.GroupBy = {'contrast'};
end

Expand Down
4 changes: 2 additions & 2 deletions src/preproc/utils/getAcquisitionTime.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

acquisitionTime = computeAcquisitionTime(sliceOrder, repetitionTime);

% ceil to avoid making this too brittle
if any(sliceOrder > ceil(acquisitionTime * 100) / 100)
% add small time buffer (0.01 s) to avoid making this too brittle
if any(sliceOrder > acquisitionTime + 0.01)
sliceOrder = bids.internal.create_unordered_list(num2str(sliceOrder));
msg = sprintf(['Acquisition time cannot be < to any slice timing value:\n\n', ...
'Current values:', ...
Expand Down
15 changes: 15 additions & 0 deletions src/stats/utils/endsWithRunNumber.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
function value = endsWithRunNumber(contrastName)
%
% USAGE::
%
% endsWithRunNumber(contrastName)
%
%
% Returns true if the contrast name ends with an underscore
% followed by some number.
%

% (C) Copyright 2024 bidspm developers
result = regexp(contrastName, '_[0-9]+\${0,1}$', 'match');
value = ~isempty(result);
return
25 changes: 10 additions & 15 deletions src/workflows/stats/bidsResults.m
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,14 @@
%
% Only necessary
% if the user did not specify the run number in result.name
% by adding an "_[0-9]*" to indicate the run number to get this contrast
% by adding an "_[0-9]+" to indicate the run number to get this contrast
% for example
%
% opt.result.name = 'listening_1'
%

endsWithRunNumber = regexp(contrastName, '_[0-9]*\${0,1}$', 'match');
if isempty(endsWithRunNumber)
tmp.name = [contrastName '_[0-9]*'];
else
tmp.name = [contrastName '_[0-9]+'];
if ~endsWithRunNumber(contrastName)
tmp.name = contrastName;
end

Expand All @@ -392,21 +390,18 @@

contrastNb = getContrastNb(tmp, opt, SPM);

constrastsNamesList = {SPM.xCon(contrastNb).name}';
contrastsNamesList = {SPM.xCon(contrastNb).name}';

for j = 1:numel(constrastsNamesList)
for j = 1:numel(contrastsNamesList)

result = opt.results(iRes);

result.name = constrastsNamesList{j};
result.name = contrastsNamesList{j};

if ~isRunLevel
% skip contrast with name ending in _[0-9]* as they are run level
% contrasts
endsWithRunNumber = regexp(result.name, '_[0-9]*$', 'match');
if ~isempty(endsWithRunNumber)
continue
end
% skip contrast with name ending in _[0-9]+
% as they are run level contrasts
if ~isRunLevel && endsWithRunNumber(result.name)
continue
end

result.space = opt.space;
Expand Down
10 changes: 5 additions & 5 deletions tests/data/models/model-default_smdl.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"Name": "default_rest_vislocalizer_vismotion_model",
"Name": "default_vislocalizer_model",
"BIDSModelVersion": "1.0.0",
"Description": "default BIDS stats model for rest/vislocalizer/vismotion task",
"Description": "default BIDS stats model for vislocalizer task",
"Input": {
"task": [
"vislocalizer"
Expand Down Expand Up @@ -61,8 +61,8 @@
},
"Software": {
"SPM": {
"InclusiveMaskingThreshold": 0.8,
"SerialCorrelation": "FAST"
"SerialCorrelation": "FAST",
"InclusiveMaskingThreshold": 0.8
}
}
},
Expand Down Expand Up @@ -107,7 +107,7 @@
"Level": "Dataset",
"Name": "dataset",
"GroupBy": [
""
"contrast"
],
"Model": {
"X": [
Expand Down
1 change: 1 addition & 0 deletions tests/tests_bids_model/test_createDefaultStatsModel.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ function test_createDefaultStatsModel_basic()

BIDS = getLayout(opt);

% opt.dir.derivatives = pwd;
opt.dir.derivatives = tempName();
opt.space = {'IXI549Space'};
opt.taskName = {'vislocalizer'};
Expand Down
49 changes: 49 additions & 0 deletions tests/tests_preproc/utils/test_getAcquisitionTime.m
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,52 @@ function test_getAcquisitionTime_bug_967()
getAcquisitionTime(sliceOrder, repetitionTime);

end

function test_openneuro_ds000224()
% test a less brittle version of getAcquisitionTime

repetitionTime = 2.2;

sliceOrder = [
1.105, ...
0, ...
1.1675, ...
0.0625, ...
1.2275, ...
0.1225, ...
1.29, ...
0.185, ...
1.35, ...
0.245, ...
1.4125, ...
0.3075, ...
1.4725, ...
0.37, ...
1.535, ...
0.43, ...
1.595, ...
0.4925, ...
1.6575, ...
0.5525, ...
1.7175, ...
0.615, ...
1.78, ...
0.675, ...
1.84, ...
0.7375, ...
1.9025, ...
0.7975, ...
1.965, ...
0.86, ...
2.025, ...
0.92, ...
2.0875, ...
0.9825, ...
2.1475, ...
1.0425];

acquisitionTime = getAcquisitionTime(sliceOrder, repetitionTime);

assertElementsAlmostEqual(acquisitionTime, 2.1389, 'absolute', 1e-4);

end
19 changes: 19 additions & 0 deletions tests/tests_stats/utils/test_endsWithRunNumber.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
function test_suite = test_endsWithRunNumber %#ok<*STOUT>
% (C) Copyright 2022 bidspm developers
try % assignment of 'localfunctions' is necessary in Matlab >= 2016
test_functions = localfunctions(); %#ok<*NASGU>
catch % no problem; early Matlab versions can use initTestSuite fine
end
initTestSuite;
end

function test_endsWithRunNumber_basic()

assertEqual(endsWithRunNumber('foo_1'), true);
assertEqual(endsWithRunNumber('foo_1_'), false);
assertEqual(endsWithRunNumber('foo_1_a'), false);
assertEqual(endsWithRunNumber('foo_'), false);
assertEqual(endsWithRunNumber('foo'), false);
assertEqual(endsWithRunNumber('foo1'), false);

end

0 comments on commit bf2e35b

Please sign in to comment.