search

pillowlab / neuroGLM

Fitting and analysis of trial-based neural spike responses with Generalized Linear Model (GLM).
MIT License
68 stars 38 forks source link

Offset of spike train forced to 1 #14

Open lachioma opened 4 years ago

lachioma commented 4 years ago

The function buildGLM.addCovariateSpiketrain might have a bug. It forces the offset to be 1 (line 11), disregarding the value in the basisStruct and without taking into account the unit of time or the bin size used. For example, if the unit of time is set to be seconds, the offset will be forced to be 1 second. Or, if another offset value has been set using basis functions, it will not be considered.

A solution to this issue could be:

% offset = 1; % Make sure to be causal. No instantaneous interaction allowed.
offset = basisStruct.param.nlOffset;
memming commented 4 years ago

I agree! Thanks for catching!

lachioma commented 4 years ago

No problem!

Actually, regarding the unit of time, I think it is important to point out that the functions buildGLM.addCovariateTiming, buildGLM.addCovariateBoxcarand buildGLM.addCovariateRaw (which in turn call the function buildGLM.addCovariate) want the input offset in ms even if you have unitOfTime = 's'.

Furthermore, I add issues using basisFactory.makeNonlinearRaisedCos with unitOfTime = 's' so I changed its line 32 in 

%iht = (0:binSize:mxt)';
iht = (0:binSize:mxt)'/binSize;

and I provide the inputs endPoints and nlOffsetin ms.

memming commented 4 years ago

@lachioma I'm very grateful for your feedback. If you can send a pull-request, I'll merge it.