search

OxIonics / ionics_fits

Small python fitting library with an emphasis on Atomic Molecular and Optical Physics
Apache License 2.0
1 stars 0 forks source link

Fix calculation of y0' initial value in sine2 model #179

Closed mbirtwell closed 7 months ago

mbirtwell commented 7 months ago

This updates the calculation of y0' to be consistent with the comment above.

I've not tested it.

hartytp commented 7 months ago

Thanks @mbirtwell

Being explicit: I think this PR should have no functional change; it's just making the code consistent with the comments, right?

This should already be tested by https://github.com/OxIonics/ionics_fits/blob/673de9426c7fa949e995ff4039d363c8683f8fc0/test/test_sinusoid.py#L67

However, looking at that I'm a little suspicious since heuristic_tol has been set to zero. That's something I sometimes do during testing to deliberately make the test fail and trigger the plotting code (I haven't got around to adding a more flexible test interface to allow plotting even when tests don't fail). So I'm surprised that this test passes at all.

hartytp commented 7 months ago

However, looking at that I'm a little suspicious since heuristic_tol has been set to zero. That's something I sometimes do during testing to deliberately make the test fail and trigger the plotting code (I haven't got around to adding a more flexible test interface to allow plotting even when tests don't fail). So I'm surprised that this test passes at all.

aah, sorry, that's right - this test really should be exact to within rounding error since it's just checking we can transfer heuristics from a sine to a sine2 model. So this is as expected

mbirtwell commented 7 months ago

Being explicit: I think this PR should have no functional change; it's just making the code consistent with the comments, right?

I think that this does change the behaviour. a' = 0.5 * a so we were previously calculating y0' = y0 + a and we're now calculating y0' = y0 + 0.5 * a.