AverageLearner1D added #283
Conversation
The AverageLearner1D has been added to the master branch (two changes were made: Learner1D._update_data_structures() does not exist anymore, so we modified AverageLearner1D accordingly; also, self._data was changed to self.data). Tutorial notebook added (tutorial_averagelearner1d_aux.py contains some auxiliary functions for the tutorial). The __init__.py files of adaptive/ and learner/, and notebook_integration.py were modified to include the AverageLearner1D.
|
This is really great! I am super excited to use this I have made some style changes and I left some small comments. Todo:
|
| _rescaled_error_in_mean and therefore will not be resampled. | ||
| - {x_i: [y_i0, y_i1, ...]} (all data samples at each point).""" | ||
| # If data_samples is given: | ||
| if isinstance(ys[0], list): |
AlvaroGI
Jun 21, 2020
Author
The current implementation of tell_many() only works if the elements of ys are lists. Is that a problem?
| This parameter controls the resampling condition. A point is resampled | ||
| if its uncertainty is larger than delta times the smallest neighboring | ||
| interval. | ||
| We strongly recommend 0 < delta <= 1. |
AlvaroGI
Jun 20, 2020
•
Author
Not really. If delta<0, the algorithm will only resample existing points and stop sampling new ones. If delta>1, the algorithm will mostly sample new points (larger delta means less resampling). These two behaviors are not commonly desired, but one may use them in some very specific cases. That is why I decided to allow for any value of delta, although in general we recommend 0<delta<1 (more specifically, delta~0.2).
|
Awesome! |
The old version was bugged (it rewrote learner data and raised errors when ran repeatedly). The new version worked well in all tests we perfomed. Doc-string also fixed.
| self._oldscale = deepcopy(self._scale) | ||
|
|
||
| # If data is given: | ||
| else: |
AlvaroGI
Jun 22, 2020
Author
This functionally allows to tell the learner about data that should be considered as "reliable", in the sense that it will not be resampled again. Theoretically, this data would correspond to the actual value of the function without noise, that is why there is no need to resample those points.
This functionality is not strictly necessary, but I was thinking in the following use-case scenario. Assume that you had some reliable previous data with low enough error bars that do not need resampling. If you want to add this data to the learner but you don't have access to all the samples anymore (only to the average value at each point), you can use this functionality, and the adaptive algorithm will take them into consideration when choosing a new point but will not resample them. I agree that this is not a common scenario but it may be useful for someone.
basnijholt
Jun 22, 2020
Member
In any case, using tell_many to indicate that a point shouldn't be sampled anymore isn't a good API. If that method desired, there should probably be another method doing that.
akhmerov
Jun 23, 2020
Contributor
I agree with @basnijholt: this feature changes what the function means, and we shouldn't do this.
We may separately consider if driving the learner like this is a relevant feature to already implement, but it certainly shouldn't be here.
AlvaroGI
Jun 30, 2020
Author
Makes sense! I removed this functionality in the last implementation of tell_many() .
| in self.data, and the previous self.data[0] and self.data[1] will be erased. | ||
| """ | ||
| # Check that all x are within the bounds | ||
| assert np.prod([x>=self.bounds[0] and x<=self.bounds[1] for x in xs]), 'x value out of bounds: remove x or enlarge the bounds of the learner' |
akhmerov
Jun 23, 2020
Contributor
This may not be an assert: assertions are for internal sanity checks. Instead we should raise ValueError here.
basnijholt
Jul 9, 2020
Member
In other learners one is able to have numbers outside of the domain, the only thing that should happen is that it doesn't suggest points outside of the domain.
This method now matches the definition from the BaseLearner. It provides a computational efficiency in some scenarios (see the comments in the code), otherwise it just performs a loop with a tell(x,y)
The docs and tutorial have been updated to include the AverageLearner1D. The previous tutorial (Python notebook) has been replaced by new tutorial as .rst file.
The AverageLearner1D has been added to the master branch (original code with design details and tests can be found here).
Tutorial notebook added (tutorial_averagelearner1d_aux.py contains some auxiliary functions for the tutorial).
Changes in existing files
The init.py files of adaptive/ and learner/, and notebook_integration.py were only modified to include the AverageLearner1D.