Bug/jon/gp update

[jonpvandermause]

Closes #70
Also makes gp.par=True the default in the otf parser, making it more efficient to reconstruct large gp models from otf output files.

[stevetorr]

Please forgive + correct me if this is wrong: But does this not create the danger that the training set is updated, but the l_mat and alpha attributes of the GP are not updated (where alpha is later used for prediction)?

[stevetorr]

If my above comment is correct, an alternative would be to push the responsibility onto e.g. an OTF or GP from AIMD training process, or we could include a quick check in the predict method to see if the length of the training set is appropriate for the size of the alpha, and if not, to then call set_L_alpha (or, if it is working, update_L_alpha)

[jonpvandermause]

That would only happen if the gp object is used incorrectly. update_db is for updating the training database, set_L_alpha and update_L_alpha are for updating the covariance matrix and related vectors.

[jonpvandermause]

Right, responsibility should be on the user, it's not worth a several order of magnitude reduction in speed for training large GPs.

[jonpvandermause]

BTW, gp_from_aimd, otf, and otf_parser all call set_L_alpha immediately after update_db, so this fix also eliminates redundant calls to set_L_alpha.

[stevetorr]

That would only happen if the gp object is used incorrectly. update_db is for updating the training database, set_L_alpha and update_L_alpha are for updating the covariance matrix and related vectors.

I understand what the methods do on their own, I am only pointing out the risk that users who are less familiar with the code might not realize that the predict method will not immediately work as they think it will if they are working outside of OTF, GP_from_AIMD, or OTF_Parser.

Right, responsibility should be on the user, it's not worth a several order of magnitude reduction in speed for training large GPs.

To be clear, I don't disagree with the fix you made, it does make things a lot faster. I am clarifying that I'm understanding the implications for potential user confusion/unexpected behavior correctly.

Also, length calls for lists or sizes of numpy arrays are made in constant time, so adding a line to predict to double-check that alpha is the same length would add a negligible amount to the runtime compared to e.g. the get_kernel_vector method.

[codecov-io]

Codecov Report

Merging #71 into master will increase coverage by 0.69%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   46.95%   47.65%   +0.69%     
==========================================
  Files          37       37              
  Lines        5641     5643       +2     
==========================================
+ Hits         2649     2689      +40     
+ Misses       2992     2954      -38

Impacted Files	Coverage Δ
flare/otf_parser.py	93.38% <ø> (ø)	⬆️
flare/gp.py	89.4% <50%> (+3.4%)	⬆️
flare/gp_from_aimd.py	87.71% <50%> (-0.78%)	⬇️
flare/gp_algebra.py	58.44% <0%> (+22.07%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0b6ba8...059ec02. Read the comment docs.

[stevetorr]

Feel free to axe the constant time check; either way, I approve of this PR, pull the trigger based on how you feel about the check.