After my latest commit, gsa crashes with octave. It turns out that the corrcoef function in octave is different from natlab, in that no second output argument is given (the pvalue of the corr-coef).
Now, there is a forge package for octave
http://octave.sourceforge.net/nan/function/corrcoef.html
that contains such an extended corrcoef for octave. Could we include that function is we are using octave?
Designs
Child items ...
Show closed items
Linked items 0
Link issues together to show that they're related.
Learn more.
We have to decide if we add the whole nan package (which includes corrcoeff routine) or only the routine we need. If possible I would prefer not to add things we do not need...
I agree. Let's add only that one function in missing
Stéphane Adjemian writes:
We have to decide if we add the whole nan package (which includes corrcoeff routine) or only the routine we need. If possible I would prefer not to add things we do not need...
@houtanb thanks a lot.
I think we need to add also function
tcdf
from nan package, otherwise at lines 324-338 of corrcoef, the significance level will not be computed. I think we could add tcdf to distributions (in the end it can be useful to have the student's cdf available).
I'm not entirely convinced by the solution that has been adopted here. Now if you don't have the nan package, then you get a warning at every run of Octave (saying that corrcoef and tcdf are overriden). And using the nan package is a bit problematic because it changes the behavior of many core Octave functions.
So I would rather prefer a solution that does not rely on the nan package, and that avoids the warning mentioned above. That could be done by putting new functions corrcoef2 and tcdf2 under missing/octave, and have a conditional in GSA to select those versions when under Octave. I can do it if you agree.
@stepan-a I think it's possible to hide all those warnings but then you lose a valuable source of information about potential bugs (e.g. when you have a spurious file in your current directory that hides a Dynare file).
And, independently of the issue of warnings, I think that we should not recommend the usage of the nan package (it overrides many built-in functions with new implementations that ignore NaNs in vectors, and therefore changes the behavior of Octave). Even the implementation of corrcoef and tcdf that have been imported from the package are probably not equivalent to the MATLAB versions because of their handling of NaNs.
By the time @houtanb pushed this corrcoef fix I was putting together a softer fix that only required tcdf.
Namely, if within octave, the standard octave corrcoef would be called with one output argument and the pvalue computation was coded within the gsa routine using tcdf.
If I remember well, using only tcdf did not provide any warning on nan package and was much cleaner.
Anyhow, I would also be OK for the new coffcoef2 and tcdf2 option.
To me, @rattoma's solution seems to be the best. You could have it in another function, using the isoctave variable to determine whether to use your new code or whether to call matlab's corrcoef.