Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.12%. Comparing base (ec4407d) to head (e597d7d).
Report is 385 commits behind head on main.

@@            Coverage Diff             @@
##             main    #6997      +/-   ##
==========================================
+ Coverage   87.78%   92.12%   +4.33%     
==========================================
  Files         100      100              
  Lines       16896    16911      +15     
==========================================
+ Hits        14832    15579     +747     
+ Misses       2064     1332     -732     

After seeing this there's a couple things that make me prefer the option of passing nu as an optional kwarg to HSGP.__init__. Mostly am concerned that Periodic HSGP #6877 won't fit within this pattern, which is a bit difficult because it's structured differently itself (an approximate spectral density and a different basis). Also it's a bit unfortunate that the docstring is split, so now it ends up being mostly boilerplate for the t-process version.

What about nu as a kwarg for HSGP, with a default of None? And use the _base_dist idea that you have to set it. The docstring of HSGP can explain the nu argument. Then we could add a function to be an alias for HSTP, like

def HSTP(mean_func, cov_func, nu):
    """ Hilbert Space Student-t process

    Reference: ...
    """
    return HSGP(...)

Then we don't need to think about an inheritance structure or refactor again so it works with HSGP Periodic, especially because it'd be great to have HSTP Periodic.

RE _base_dist, could it be anything? I would expect it'd only be Normal or StudentT. Might be nice to be explicit about that.

But I'm super excited to use this, I think HSTP will be used as a default before HSGP in a lot of cases. What do you think @ferrine?

Let's wait the #6883 and then continue, so it will be one round of refactoring

Let's wait the #6883 and then continue, so it will be one round of refactoring

Are you sure? Pytensorifying the GP and covariance function library is a huge refactor. I think that it'd be great to get HSTP's in ASAP, and a refactor isn't necessary to do so (see my comment from Nov 7 above here).

I agree with @bwengals, it might take a while to do the other changes.

Still not sure what should be the design choice, and should this eve stop me from taking any branch I find more appealing. We'll anyway refactor everything as far as I get it

What about just keeping the _base_dist idea like you have, and have nu be a optional kwarg to HSGP? Then _base_dist can either be StudentT if nu is not None or Normal if nu is None. That would keep it all within HSGP, and you could just modify the docstring once there. I think it'd make HSTPs easier for users to discover. Then if you wanted to make an quick alias for HSTP you could. wdyt?

After all the refactorings in HSGP, it makes sense to create the PR from scratch