search

LLNL / MuyGPyS

A fast, pure python implementation of the MuyGPs Gaussian process realization and training algorithm.
Other
23 stars 11 forks source link

Reorganize hyperparameters and init hierarchical nonstationary hyperparameter #129

Closed igoumiri closed 1 year ago

igoumiri commented 1 year ago

Since I was about to introduce a third type of hyperparameter, I moved them all to a new directory and I renamed Hyperparameter to ScalarHyperparameter. I added a stub of HierarchicalNonstationaryHyperparameter based on https://github.com/LLNL/MuyGPyS/issues/71 and I figured I'd rather do incremental small PRs so I'm putting this one out.

Some of the files got auto-formatted since I run black on save.

igoumiri commented 1 year ago

Hmm, there is a circular dependency because the hierarchical hyperparameter needs a kernel… Maybe it should receive the kernel itself in __init__ rather than the kwargs?

bwpriest commented 1 year ago

Hmm, there is a circular dependency because the hierarchical hyperparameter needs a kernel… Maybe it should receive the kernel itself in __init__ rather than the kwargs?

Also yes, circular dependencies aside I believe that HierarchicalNonstationaryHyperparameter should receive an already-initialized kernel instead of the kwargs to be used to create one. This will be consistent with the changes introduced in PR #118, and make it easy to swap in different kernels without modifying the source code.

bwpriest commented 1 year ago

Some of the files got auto-formatted since I run black on save.

I think that you need to add the arguments --line-length 80 to your black invocations so that all of our autoformats agree. If you are using vscode, you need to add the lines --line-length and 80 to Python › Formatting: Black Args in your settings. If you are using a different solution or you need help, please let me know.

alecmdunton commented 1 year ago

I like the introduce of a separate directory for hyperparameter objects - this could give me a chance to introduce a call method which would clean up the way we handle isotropy/anisotropy in the current implementation I just pushed.