Smooth and stepped piecewise arrhenius - #215
Conversation
Addition of a piecewise arrhenius function and a class that allow to use it accordingly. Addition of a smoothed piecewise arrhenius function and a class using a sigmoid function. Both cases are setup and documented following what was done with the other arrhenius classes and functions.
|
@Lorentyfle, thanks for opening this PR. Would it be possible to include tests to go along with the new functionality? |
bounds was exchange with priors and a test function was added for it.
|
@arm61, of course! I also saw that all TemperatureDependent changed from using bounds to priors, so I adapted my functions to be in agreement with this new formalism. |
| super().__init__(diffusion, piecewise_smooth_equation, parameter_names, parameter_units, priors=priors) | ||
|
|
||
| @property | ||
| def activation_energy_low(self) -> VariableLike | Samples: |
There was a problem hiding this comment.
Are these the best variable names? Genuinely asking. Is there something that shows up in the literature?
There was a problem hiding this comment.
I was thinking of keeping 'activation_energy' while adding 'low' and 'high' to show which temperature section it is. But it is true that '_low_temperature' or '_high_temperature' could also work and be far more descriptive.
In literature, it seems the transition temperature is also called Arrhenius Break:
https://pmc.ncbi.nlm.nih.gov/articles/PMC7959718/
Can be stated as "temperature segment":
https://www.osti.gov/servlets/purl/2333851
But low-temperature and high-temperature seem to also be valid:
https://link.springer.com/chapter/10.1007/978-94-009-5167-9_32
I think the latest one is better ('activation_energy_high_temperature' and 'activation_energy_low_temperature') as it would be easier to understand which activation energy it is, instead of stating a numerical description, or the condensed version ('activation_energy_low' and 'activation_energy_high'), and would indicate which location at a glance this activation energy is from.
|
Just a few minor documentation things then we can get this merged. |
|
Also, looks like there are some linting errors. If you run:
These should go away. |
Corrected some more issues in the change of name. Make the precision of the expected results better. Decrease the value of the width for the test of the values as it will change the one extremity of the piecewise result making it less accurate using the current method for computation of the expected result. If failure will happen again, a more formal computation using the impact of the sigmoid will be done.
A more formal computation using the impact of the sigmoid is done for the computation of the expected calculation that was done by hand.
arm61
left a comment
arm61
left a comment
There was a problem hiding this comment.
I prefer *_temperature. Always better to be verbose. I will merge now.
|
Thanks for the contribution @Lorentyfle! |
2 participants
Addition of a piecewise Arrhenius function and a class that allow to use it accordingly. Addition of a smoothed piecewise Arrhenius function and a class using a sigmoid function.
Both cases are set up and documented following what was done with the other Arrhenius classes and functions.