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

BUG: Resampled column has one too many rows #358

Closed
wants to merge 2 commits into from

Conversation

effigies
Copy link
Collaborator

Here's a test to demonstrate what I described in #355. There's probably a way to do this without a simulated layout, but hopefully this demonstrates the issue in a plausible context.

I don't understand the cause yet. @tyarkoni Please let me know if it's obvious to you.

layout = BIDSLayout('.', validate=False)
coll = load_variables(layout).get_collections('run')[0]
dense_var = coll.variables['val'].to_dense(coll.sampling_rate)
regressor = dense_var.resample(1.0 / TR).values
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably note that you do get the right shape if you go directly to 1/TR:

regressor = coll.variables['val'].to_dense(1.0 / TR).values

@effigies
Copy link
Collaborator Author

It's possible that the "bug" is simply sampling to freq A and then to freq B does not necessarily produce the same shape as sampling directly to freq B, which is fine, but what we really need to do is in methods that take sampling_rate='TR', we should additionally ensure that n_rows = n_vols.

@codecov
Copy link

codecov bot commented Jan 29, 2019

Codecov Report

Merging #358 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
+ Coverage   73.27%   73.34%   +0.07%     
==========================================
  Files          24       24              
  Lines        2604     2604              
  Branches      640      640              
==========================================
+ Hits         1908     1910       +2     
+ Misses        513      511       -2     
  Partials      183      183
Flag Coverage Δ
#unittests 73.34% <ø> (+0.07%) ⬆️
Impacted Files Coverage Δ
bids/layout/layout.py 76.05% <0%> (+0.56%) ⬆️

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 e9fc2df...f936aed. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jan 29, 2019

Codecov Report

Merging #358 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
+ Coverage   73.27%   73.34%   +0.07%     
==========================================
  Files          24       24              
  Lines        2604     2604              
  Branches      640      640              
==========================================
+ Hits         1908     1910       +2     
+ Misses        513      511       -2     
  Partials      183      183
Flag Coverage Δ
#unittests 73.34% <ø> (+0.07%) ⬆️
Impacted Files Coverage Δ
bids/layout/layout.py 76.05% <0%> (+0.56%) ⬆️

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 e9fc2df...f936aed. Read the comment docs.

@effigies
Copy link
Collaborator Author

Well, if that was a fix, it wasn't the only fix needed.

[gw0] linux2 -- Python 2.7.15 /home/travis/build/bids-standard/pybids/venv/bin/python
tmpdir = local('/tmp/pytest-of-travis/pytest-0/popen-gw0/test_resampling_edge_case_2_000')
TR = 2.000001, nvols = 251
    @pytest.mark.parametrize(
        "TR, nvols",
        [(2.00000, 251),
         (2.000001, 251)])
    def test_resampling_edge_case(tmpdir, TR, nvols):
        tmpdir.chdir()
        os.makedirs('sub-01/func')
        with open('sub-01/func/sub-01_task-task_events.tsv', 'w') as fobj:
            fobj.write('onset\tduration\tval\n1\t0.1\t1\n')
        with open('sub-01/func/sub-01_task-task_bold.json', 'w') as fobj:
            json.dump({'RepetitionTime': TR}, fobj)
    
        dataobj = np.zeros((5, 5, 5, nvols), dtype=np.int16)
        affine = np.diag((2.5, 2.5, 2.5, 1))
        img = nb.Nifti1Image(dataobj, affine)
        img.header.set_zooms((2.5, 2.5, 2.5, TR))
        img.to_filename('sub-01/func/sub-01_task-task_bold.nii.gz')
    
        layout = BIDSLayout('.', validate=False)
        coll = load_variables(layout).get_collections('run')[0]
        dense_var = coll.variables['val'].to_dense(coll.sampling_rate)
>       regressor = dense_var.resample(1.0 / TR).values
/home/travis/build/bids-standard/pybids/bids/variables/tests/test_variables.py:203: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
bids/variables/variables.py:439: in resample
    ???
bids/variables/variables.py:454: in resample
    ???
/home/travis/build/bids-standard/pybids/venv/lib/python2.7/site-packages/scipy/interpolate/interpolate.py:433: in __init__
    _Interpolator1D.__init__(self, x, y, axis=axis)
/home/travis/build/bids-standard/pybids/venv/lib/python2.7/site-packages/scipy/interpolate/polyint.py:60: in __init__
    self._set_yi(yi, xi=xi, axis=axis)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <scipy.interpolate.interpolate.interp1d object at 0x7f3fa0d32f18>
yi = array([0, 0, 0, ..., 0, 0, 0])
xi = array([   0,    1,    2, ..., 5018, 5019, 5020]), axis = -1
    def _set_yi(self, yi, xi=None, axis=None):
        if axis is None:
            axis = self._y_axis
        if axis is None:
            raise ValueError("no interpolation axis specified")
    
        yi = np.asarray(yi)
    
        shape = yi.shape
        if shape == ():
            shape = (1,)
        if xi is not None and shape[axis] != len(xi):
>           raise ValueError("x and y arrays must be equal in length along "
                             "interpolation axis.")
E           ValueError: x and y arrays must be equal in length along interpolation axis.
/home/travis/build/bids-standard/pybids/venv/lib/python2.7/site-packages/scipy/interpolate/polyint.py:125: ValueError

@adswa
Copy link
Contributor

adswa commented Jan 29, 2019

Not sure if its relevant, but let me share some of the obscure observations I made (without the change in f936aed) with this issue:
I've tried it on slimmed directories with a) 3 subjects and 2 runs each and b) 3 subjects and 3 runs each.

  • I can't trigger this error if I use a model with only one event anywhere.
(i.e. this model):
{
    "name": "FEF_localizer",
    "Input": {
        "session": "movie"
        },
    "Steps": [
        {
            "Level": "run",
            "Model": {
                "X": [
                    "amplitude_.RIGHT"
                    ]
            },
            "Contrasts": [],
            "AutoContrasts": true,
            "Transformations": [{
                "Name": "Split",
                "Input": ["amplitude_"],
                "By": ["trial_type"]
                },
                {
                "Name": "Convolve",
                "Input": ["amplitude_.RIGHT"],
                "Model": "spm"
                  }]
            },
        {
            "Level": "subject",
            "AutoContrasts": true
        },
        {
            "Level": "dataset",
            "AutoContrasts": true
        }]
}

  • if I include a second variable it fails only for the slightly larger directory (3 subjects, 3 runs) and runs fine on the slightly slimmer one (3 subjects, 2 runs).
(i.e. this model:)
{
    "name": "FEF_localizer",
    "Input": {
        "session": "movie"
        },
    "Steps": [
        {
            "Level": "run",
            "Model": {
                "X": [
                    "amplitude_.RIGHT",
                    "amplitude_.LEFT"
                    ]
            },
            "Contrasts": [],
            "AutoContrasts": true,
            "Transformations": [{
                "Name": "Split",
                "Input": ["amplitude_"],
                "By": ["trial_type"]
                },
                {
                "Name": "Convolve",
                "Input": ["amplitude_.RIGHT", "amplitude_.LEFT"],
                "Model": "spm"
                  }]
            },
        {
            "Level": "subject",
            "AutoContrasts": true
        },
        {
            "Level": "dataset",
            "AutoContrasts": true
        }]
}

Traceback for the failure:
190129-10:33:21,320 nipype.workflow WARNING:
	 [Node] Error on "fitlins_wf.loader" (/home/adina/Documents/MasterMagdeburg/USA_Dartmouth/content/localizer_project/BIDS_sacc/Jan_29_2ev_wd/fitlins_wf/loader)
190129-10:33:23,119 nipype.workflow ERROR:
	 Node loader failed to run on host odin.
190129-10:33:23,119 nipype.workflow ERROR:
	 Saving crash info to /home/adina/Documents/MasterMagdeburg/USA_Dartmouth/content/localizer_project/BIDS_sacc/crash-20190129-103323-adina-loader-e8a1d682-f64c-46f9-ba51-680f9f68a132.pklz
Traceback (most recent call last):
  File "/home/adina/Repos/nipype/nipype/pipeline/plugins/multiproc.py", line 69, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 473, in run
    result = self._run_interface(execute=True)
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 557, in _run_interface
    return self._run_command(execute)
  File "/home/adina/Repos/nipype/nipype/pipeline/engine/nodes.py", line 637, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/home/adina/Repos/nipype/nipype/interfaces/base/core.py", line 369, in run
    runtime = self._run_interface(runtime)
  File "/home/adina/Repos/fitlins/fitlins/interfaces/bids.py", line 181, in _run_interface
    self._load_level1(runtime, analysis)
  File "/home/adina/Repos/fitlins/fitlins/interfaces/bids.py", line 195, in _load_level1
    for sparse, dense, ents in step.get_design_matrix():
  File "/home/adina/Repos/pybids/bids/analysis/analysis.py", line 263, in get_design_matrix
    for n in nodes]
  File "/home/adina/Repos/pybids/bids/analysis/analysis.py", line 263, in <listcomp>
    for n in nodes]
  File "/home/adina/Repos/pybids/bids/analysis/analysis.py", line 402, in get_design_matrix
    sampling_rate=sampling_rate, **kwargs)
  File "/home/adina/Repos/pybids/bids/variables/kollekshuns.py", line 348, in to_df
    in_place=False).values())
  File "/home/adina/Repos/pybids/bids/variables/kollekshuns.py", line 279, in resample
    kind=kind)
  File "/home/adina/Repos/pybids/bids/variables/variables.py", line 439, in resample
    var.resample(sampling_rate, True, kind)
  File "/home/adina/Repos/pybids/bids/variables/variables.py", line 454, in resample
    f = interp1d(x, self.values.values.ravel(), kind=kind)
  File "/usr/local/lib/python3.5/dist-packages/scipy/interpolate/interpolate.py", line 433, in __init__
    _Interpolator1D.__init__(self, x, y, axis=axis)
  File "/usr/local/lib/python3.5/dist-packages/scipy/interpolate/polyint.py", line 60, in __init__
    self._set_yi(yi, xi=xi, axis=axis)
  File "/usr/local/lib/python3.5/dist-packages/scipy/interpolate/polyint.py", line 125, in _set_yi
    raise ValueError("x and y arrays must be equal in length along "
ValueError: x and y arrays must be equal in length along interpolation axis.

Maybe relevant facts:

  • there is an unequal number of right and left events (for context, these 'right', 'left', 'up', 'down' are gaze directions during movie watching)
  • the number of volumes for the runs are:
nib-ls sub-01/ses-movie/func/sub-01_ses-movie_task-avmovie_run-*_space-MNI152NLin6Sym_desc-highpass_bold.nii.gz
sub-01/ses-movie/func/sub-01_ses-movie_task-avmovie_run-1_space-MNI152NLin6Sym_desc-highpass_bold.nii.gz float32 [ 68,  84,  58, 451] 2.50x2.50x2.50x2.00
sub-01/ses-movie/func/sub-01_ses-movie_task-avmovie_run-2_space-MNI152NLin6Sym_desc-highpass_bold.nii.gz float32 [ 68,  84,  58, 441] 2.50x2.50x2.50x2.00
sub-01/ses-movie/func/sub-01_ses-movie_task-avmovie_run-3_space-MNI152NLin6Sym_desc-highpass_bold.nii.gz float32 [ 68,  84,  58, 438] 2.50x2.50x2.50x2.00

I'm pushing the slightly larger dataset to branch slim2 on hydra if you want to take a peek. The model with two events is in models/model_v3_smdl_autocon_splitconv_twoevents.json and I call it like this: fitlins . Jan_29_2ev_2runs 'run' -m models/model_v3_smdl_autocon_splitconv_twoevents.json --desc highpass --space 'MNI152NLin6Sym' -d $PWD -w 'Jan_29_2ev_2runs_wd' --n-cpus 3

@tyarkoni
Copy link
Collaborator

I'd be kind of surprised if this has anything to do with the event structure per se... I think what's probably going on is that only some models have timing information that's causing rounding irregularities.

I do plan to look at this and come up with a fix, but am deferring all non-essential work until I have a draft of 0.8 pushed.

@adelavega
Copy link
Collaborator

Closed by #365

@adelavega adelavega closed this Feb 10, 2019
@effigies effigies deleted the bug/resampling_error branch February 10, 2019 21:39
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.

4 participants