LLNL / MuyGPyS

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

Feature/fast regression tutorial #63

Closed alecmdunton closed 1 year ago

alecmdunton commented 1 year ago

Added fast regression tutorial notebook.

bwpriest commented 1 year ago

Everything up to and including the hyperparameter training is the same as docs/examples/univariate_regression_tutorial.ipynb. In order to make clear what is interesting about this tutorial, I would suggest removing the expository markdown and collapsing all of that code into one cell, while adding a markdown cell that explains that all of that code is the same as what is in docs/examples/univariate_regression_tutorial.ipynb. This will help the user to focus on what is actually important and different about this tutorial.

bwpriest commented 1 year ago

As a general comment, you have a lot of lines of code that are really long. I would break them up into narrower chunks so that they will be easier to read. For example, I would refactor

fast_predictions_bayes = muygps_bayes.fast_regress_from_indices(np.arange(0,test_count), closest_set,test_features, train_features, closest_neighbor, precomputed_coefficients_matrix_bayes)

into

fast_predictions_bayes = muygps_bayes.fast_regress_from_indices(
    np.arange(0,test_count),
    closest_set,
    test_features,
    train_features,
    closest_neighbor,
    precomputed_coefficients_matrix_bayes
)

People are lazy readers, especially of code. If they have to scroll horizontally to read a line of code, then they will just not do so.

bwpriest commented 1 year ago

the JAX correctness checks failed because of

ImportError: cannot import name '_muygps_fast_nn_update' from 'MuyGPyS._src.gp.muygps.numpy'

tests/jax_correctness.py needs to be updated with the changes you made to MuyGPyS._src.

bwpriest commented 1 year ago

I would add a markdown cell at the end, below the timing, where you comment on the improvement. It might be worth mentioning that the improvement is even better when JAX accelerated, as well as anything else you can think of. It is a good place to brag.

bwpriest commented 1 year ago

Looks good. Mark as ready when you are happy with it.