Conversation
|
The scaling of the logistic function is still quite "wilde". |
znichollscr
left a comment
There was a problem hiding this comment.
Nice. Let's please add the extra tests suggested so we have a record of how this behaves in practice, then we should be good to merge
| + include support for logistic-decay | ||
| + we added three new classes/functions to the API in the module `convergence` | ||
| + `LogisticDecaySplineHelper` | ||
| + `LogisticDecaySplineHelperDerivative` | ||
| + `get_logistic_decay_harmonised_spline` | ||
| + whereby the function `get_logistic_decay_harmonised_spline` is mainly meant as user-interface | ||
| This function can be used as value for the `get_harmonise_spline` parameter in `harmonise`. | ||
| The function takes the same arguments as `get_cosine_decay_harmonised_spline` and additionally a | ||
| `slope` and `shift` argument. | ||
| + These arguments determine slope and location of the logistic function and can be passed using | ||
| `functools.partial(get_logistic_decay_harmonised_spline, slope = 0., shift = 0.)` | ||
| + An example use-case is added to the `getting-started-tutorial`. |
There was a problem hiding this comment.
| + include support for logistic-decay | |
| + we added three new classes/functions to the API in the module `convergence` | |
| + `LogisticDecaySplineHelper` | |
| + `LogisticDecaySplineHelperDerivative` | |
| + `get_logistic_decay_harmonised_spline` | |
| + whereby the function `get_logistic_decay_harmonised_spline` is mainly meant as user-interface | |
| This function can be used as value for the `get_harmonise_spline` parameter in `harmonise`. | |
| The function takes the same arguments as `get_cosine_decay_harmonised_spline` and additionally a | |
| `slope` and `shift` argument. | |
| + These arguments determine slope and location of the logistic function and can be passed using | |
| `functools.partial(get_logistic_decay_harmonised_spline, slope = 0., shift = 0.)` | |
| + An example use-case is added to the `getting-started-tutorial`. | |
| + Include supported for logistic-decay | |
| + Added new features to [convergence][gradient_aware_harmonisation.convergence] | |
| + [LogisticDecaySplineHelper][gradient_aware_harmonisation.convergence.LogisticDecaySplineHelper] | |
| + [LogisticDecaySplineHelperDerivative][gradient_aware_harmonisation.convergence.LogisticDecaySplineHelperDerivative] | |
| + [get_logistic_decay_harmonised_spline][gradient_aware_harmonisation.convergence.get_logistic_decay_harmonised_spline] | |
| + The function [get_logistic_decay_harmonised_spline][gradient_aware_harmonisation.convergence.get_logistic_decay_harmonised_spline] is an adapter between the underlying classes and the interface required for the `get_harmonise_spline` parameter in [harmonise][gradient_aware_harmonisation.harmonise]. | |
| The function takes the same arguments as [get_cosine_decay_harmonised_spline][gradient_aware_harmonisation.convergence.get_cosine_decay_harmonised_spline] and additional | |
| `slope` and `shift` arguments. These arguments determine the slope and location of the logistic function and can be can be 'pre-passed' to the function using [functools.partial](https://docs.python.org/3/library/functools.html#functools.partial) e.g. `harmonise(..., get_harmonise_spline=functools.partial(get_logistic_decay_harmonised_spline, slope = 0., shift = 0.)...)` | |
| + An example use-case is included in the [getting started tutorial][gradient-aware-harmonisation-getting-started]. |
Same reasoning as #16 (comment)
| gamma_decaying: int | float | NP_FLOAT_OR_INT | NP_ARRAY_OF_FLOAT_OR_INT = ( | ||
| 1 | ||
| - 1 | ||
| / ( | ||
| 1 | ||
| + np.exp( | ||
| -2 * np.exp(self.slope) * delta * x_prime | ||
| + 3 * delta | ||
| + self.shift | ||
| ) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Can you please add some tests of what happens at a few key values of x e.g.
- x is very negative
- x equal to the initial time
- x halfway between the initial and final time
- x equal to the final time
- x is very positive
Also tests that summing a spline with apply_to_convergence=True and apply_to_convergence=False gives 1 everywhere.
I'm assuming these things all hold, but it's good to check so we have the proof for ourselves, should we ever need it.
Description
Checklist
Please confirm that this pull request has done the following:
changelog/