Conversation
| """ | ||
| :param models: list of GPflow models representing our beliefs about the problem | ||
| :param optimize_restarts: number of optimization restarts to use when training the models | ||
| """ |
There was a problem hiding this comment.
What is the python way of documenting overrided subclass functions such as the constructor? I think sphinx will actually only display the doc string of the overrided method?
Here optimize_restarts is not visible from the overrided class anyways
On the other hand, many overrided functions and parameters (such as models here, or build_acquisition) do not required specific documentation (do we copy the doc string for each overrided method, or document only the base/abstract method).
There was a problem hiding this comment.
by default all special members (getitem, eq etc) are excluded. I added these special-members directive but its not doing anything. I often see constructor documentation put in place of class documentation. If you google it, there are some ways to enable but be a bit careful and check the output of the sphinx building very well. I noticed changing things in the conf.py can have significant side effects.
|
|
||
| It is called after initialization and set_data(), and before optimizing the acquisition function itself. | ||
| It is called automatically after initialization and each time set_data() is called. | ||
| When using the high-level :class:`..BayesianOptimizer` class calling set_data() is taken care of. |
There was a problem hiding this comment.
I hope I got this right. The old explanation did not seem quite right to me. The part about the high-level class could be removed as that will be explained in BayesianOptimizer itself.
|
|
||
| :param domain: :class:`.Domain` object, the input transform of the data scalers is configured as a transform | ||
| from domain to the unit cube with the same dimensionality. | ||
| from domain to the unit cube with the same dimensionality. |
There was a problem hiding this comment.
seems multiline param explanation should be properly indented for sphinx to display it correctly. Perhaps using the python line continuation \ works too
| """ | ||
| Method triggered after calling set_data(). | ||
|
|
||
| Override for pre-calculation of quantities used later in |
There was a problem hiding this comment.
It is explained as an override, for what reason would you want to call setup yourself? If not, we might make it private (deja vu, did we have that discussion before?)
There was a problem hiding this comment.
hmm, this could indeed be turned into private as it is now called from set_data. But I don't think it is very nice to present an interface for users to implement where it is a common practice to override a protected method.
| """ | ||
| AutoFlow method to compute the acquisition scores for candidates, also returns the gradients. | ||
|
|
||
| :return: (ndarray of acquisition scores(N x 1), ndarray of the gradients of the scores (N*D)) |
There was a problem hiding this comment.
this is still on my list. I see I made some typos
Codecov Report
@@ Coverage Diff @@
## master #55 +/- ##
=======================================
Coverage 99.73% 99.73%
=======================================
Files 14 14
Lines 757 757
=======================================
Hits 755 755
Misses 2 2
Continue to review full report at Codecov.
|
icouckuy
changed the title
- Added small getting started guide and first steps notebook - restructured the API menu and added Bayesian optimizer + designs pages - Went over a lot of doc strings, tried to uniformize the shape information + notation of the matrices - Added sectio headings for all notebooks (they show up as subsections in the menu) - Fixed all sphinx warnings
3 participants
I did another pass over the documentation of the acquisition function fixing typo's, added some missing doc strings, etc. Also fixed #51
I have some more question/comments which I will add as comments in the change list.