Integration and microtime

SRSteinkamp commented 1 year ago

Hi @iChaker,

sorry it's ages, but maybe you can help me out ... I am currently running in a few errors during integration, though only in one fringe case. I am trying to make it as sensible as possible...

I have a question about spm_prf_response and spm_int_sparse.

In spm_prf_response we define the max number of nbins using max(), I am just wondering if it should be always to be set to nbins. The reason is, if we are in the situation, that there are onsets after the scanner ended, we get into possible weird situations down stream.

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_prf_response.m#L89

This would mean slightly modifying the code below (spm_prf_response, ll 105-111), so that it does not try to insert indices larger than nbins:

        z = zeros(1,nbins);
        for t = 1:n    
            % Microtime index for this volume
            ind = U(t).ind;  

            resp = U(t).signals1D' .* W;
            z(ind) = z(ind) + sum(resp);

If I understand it correctly, we are downstream in spm_int_sparse calculating a factor to make the dimension of the hidden states, fit the number of time points in y:

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_int_sparse.m#L121

Where u is the number of nbins and v is the number of samples / time points in y. And if I understand it correctly, the line above compresses / extends the microtime points, so that the timesteps are aligned between the inputs u and the outputs y, even if they are not entirely aligned. The line above also results in some rounding errors in my fringe sample.

Just wondering if you think it might make sense to exclude indices for timepoints that are longer than the actually recorded signal?

iChaker commented 1 year ago

Hey Simon,

Sorry for the late reply, I have a couple of deadlines converging this week, I will reply asap.

Best, Iyadh

On Thu, Jan 5, 2023 at 11:13 AM Simon Steinkamp @.***> wrote:

Hi @iChaker https://github.com/iChaker,

sorry it's ages, but maybe you can help me out ... I am currently running in a few errors during integration, though only in one fringe case. I am trying to make it as sensible as possible...

I have a question about spm_prf_response and spm_int_sparse.

In spm_prf_response we define the max number of nbins using max(), I am just wondering if it should be always to be set to nbins. The reason is, if we are in the situation, that there are onsets after the scanner ended, we get into possible weird situations down stream.

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_prf_response.m#L89

This would mean slightly modifying the code below (spm_prf_response, ll 105-111), so that it does not try to insert indices larger than nbins:
    z = zeros(1,nbins);
    for t = 1:n
        % Microtime index for this volume
        ind = U(t).ind;

        resp = U(t).signals1D' .* W;
        z(ind) = z(ind) + sum(resp);
If I understand it correctly, we are downstream in spm_int_sparse calculating a factor to make the dimension of the hidden states, fit the number of time points in y:

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_int_sparse.m#L121

Where u is the number of nbins and v is the number of samples / time points in y. And if I understand it correctly, the line above compresses / extends the microtime points, so that the timesteps are aligned between the inputs u and the outputs y, even if they are not entirely aligned. The line above also results in some rounding errors in my fringe sample.

Just wondering if you think it might make sense to exclude indices for timepoints that are longer than the actually recorded signal?

— Reply to this email directly, view it on GitHub https://github.com/iChaker/CPM_framework/issues/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHXN7UCDZVAE4JZGKQS43QTWQ2NDJANCNFSM6AAAAAATRYVHGU . You are receiving this because you were mentioned.Message ID: @.***>

iChaker commented 1 year ago

Hey Simon,

If I remember correctly, the precomputed structure U has a boolean variable U(1).microtime that says if it supports microtime or not. saved here: https://github.com/iChaker/CPM_framework/blob/main/CPM_Toolbox/cpm_precompute.m#L122 used here: https://github.com/iChaker/CPM_framework/blob/main/CPM_Toolbox/spm_prf_response.m#L99

This line is just a generic way to pull nbins regardless of the case nbins = max(U(1).nbins, max(U(end).ind));

The program we have naturally assumes that the experimental data and the fMRI data have the same length. In the case they do not have the same length, the easiest way to fix this is to trim one or the other before doing any precomputation. You can write a utility function for this situation, instead of changing spm_prf_response which is a very volatile function.

Sorry again for the late response! I hope I understood / remembered things correctly.

Best regards, Iyadh

On Thu, Jan 5, 2023 at 11:13 AM Simon Steinkamp @.***> wrote:

Hi @iChaker https://github.com/iChaker,

sorry it's ages, but maybe you can help me out ... I am currently running in a few errors during integration, though only in one fringe case. I am trying to make it as sensible as possible...

I have a question about spm_prf_response and spm_int_sparse.

In spm_prf_response we define the max number of nbins using max(), I am just wondering if it should be always to be set to nbins. The reason is, if we are in the situation, that there are onsets after the scanner ended, we get into possible weird situations down stream.

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_prf_response.m#L89

This would mean slightly modifying the code below (spm_prf_response, ll 105-111), so that it does not try to insert indices larger than nbins:
    z = zeros(1,nbins);
    for t = 1:n
        % Microtime index for this volume
        ind = U(t).ind;

        resp = U(t).signals1D' .* W;
        z(ind) = z(ind) + sum(resp);
If I understand it correctly, we are downstream in spm_int_sparse calculating a factor to make the dimension of the hidden states, fit the number of time points in y:

https://github.com/iChaker/CPM_framework/blob/d4ffe19fa1fc2e11fb0ec972096eef282080386e/CPM_Toolbox/spm_int_sparse.m#L121

Where u is the number of nbins and v is the number of samples / time points in y. And if I understand it correctly, the line above compresses / extends the microtime points, so that the timesteps are aligned between the inputs u and the outputs y, even if they are not entirely aligned. The line above also results in some rounding errors in my fringe sample.

Just wondering if you think it might make sense to exclude indices for timepoints that are longer than the actually recorded signal?

— Reply to this email directly, view it on GitHub https://github.com/iChaker/CPM_framework/issues/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHXN7UCDZVAE4JZGKQS43QTWQ2NDJANCNFSM6AAAAAATRYVHGU . You are receiving this because you were mentioned.Message ID: @.***>

iChaker / CPM_framework

Integration and microtime #13