Use only noise OR sigma as a parameter in gp.marginal_likelihood()

[danhphan]

Hi, in pymc.gp.gp.py file, several GP classes such as "Marginal" and "MarginalApprox" use noise as a parameter in their marginal_likelihood() function, while other GP classes like "MarginalKron" use sigma as a parameter.

And also in docstrings, sometimes we use noise=sigma, sometimes we use sigma=sigma.

As the use of noise and sigma in the gp context is similar, it could be better to use ONLY noise OR sigma in these GP classes. This could make the APIs for marginal_likelihood() function a bit more consistent to users.

I would prefer only keep noise as a parameter, as it represents the meaning of noise part in the equation: y = f(x) + noise. But using only sigma is also fine.

Would love to have your opinions on this. Thanks!

[fonnesbeck]

I would tend to lean towards sigma only because it is used elsewhere in PyMC as a standard deviation parameter. If we ever change the PyMC conventions from Greek to descriptive parameter names, then noise would make sense here. That said, since GPs are a quasi-independent submodule that have unique attributes so having different parameter names is not totally unreasonable. Its a question of whether we want parameter names consistent across PyMC or just across the GP submodule.

[danhphan]

Hi, yes agree, it should be better to make it consistent for other modules in PyMC. If it is alright, I will open an issue in PyMC to change the noise parameter to sigma. It should be an easy change (if nobody takes the issue, I will do it soon). Thank you :)

[bwengals]

Great idea, I like sigma too. And it'd be good to be consistent.

One thing that should be better documented though, is I think where noise is used, you can use a scalar, implying white noise, or an actual Covariance, so any process could be considered noise. Where sigma is used, it has to be a scalar. I think that must have been the point of the different naming. It's confusing though, and it'd be better to be consistent, but documented.

So for instance here's this line in Marginal:

pymc/pymc/gp/gp.py

Line 449 in 0b191ad

if not isinstance(noise, Covariance):

And then MarginalSparse it has to be a scalar. The implementations depend on white noise. I think for VFE you can do more than white noise, but would need to double check.

[danhphan]

Yes, agree, we definitely need to update the docs to make it clearer. Many thanks @bwengals :)