Skip to content

Add logistic decay#18

Open
florence-bockting wants to merge 7 commits intoadd_polynomial_decayfrom
add_logistic_decay
Open

Add logistic decay#18
florence-bockting wants to merge 7 commits intoadd_polynomial_decayfrom
add_logistic_decay

Conversation

@florence-bockting
Copy link
Contributor

@florence-bockting florence-bockting commented Apr 3, 2025

Description

  • add logistic decay

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

@florence-bockting florence-bockting marked this pull request as ready for review April 3, 2025 10:10
@florence-bockting
Copy link
Contributor Author

florence-bockting commented Apr 3, 2025

The scaling of the logistic function is still quite "wilde".
I think I lost the focus and made it overly complicated. Need to look at it again with a fresh mind.
Suggestions for better shifting/scaling of the logistic function are welcome.

@znichollscr znichollscr changed the base branch from main to add_polynomial_decay April 7, 2025 07:51
@znichollscr znichollscr changed the base branch from add_polynomial_decay to main April 7, 2025 07:56
@znichollscr znichollscr changed the base branch from main to add_polynomial_decay April 7, 2025 07:56
Copy link
Contributor

@znichollscr znichollscr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +1 to +12
+ 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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
+ 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)

Comment on lines +748 to +759
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
)
)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants