lgtm-com[bot]
commented
3 years ago This pull request introduces 9 alerts when merging 1b7041e53fc124c290a1f6f28d68a79d7549f5ae into b38b3e9a42ea06992829e9b127b83cd0ae32652b - view on LGTM.com
new alerts:
- 4 for Unused import
- 1 for Module is imported more than once
- 1 for Unreachable code
- 1 for First argument to super() is not enclosing class
- 1 for Module is imported with 'import' and 'import from'
- 1 for Wrong number of arguments in a class instantiation
I had to update a variety of files in our codebase here and am adding two variants for how to implement the biophysical model.
My suggestion is to use the version that lives in gaussian_process_regressors.py and inherits from the GPs, and then is a plug-in replacement to variational GP.
I also edited net to use this.
For a demo on how to use this please look into scripts/biophysical/biophysical_test.py
Imho we can now train with this and I can now design the acquisition function based on this, as I also implemented sampling from deltaG.
Note: sampling from deltaG is done in a hacky way within net currently, as the overall structure of out models is suboptimal to do this optimally on short notice.
My pitch is to use this now to get our results and then reorganize post-deadline, so keep that in mind when reviewing.