fixed two minor issues in the LMFit wrapper. #301 and #302 - #303
fixed two minor issues in the LMFit wrapper. #301 and #302#303rozyczko wants to merge 3 commits into
Conversation
rozyczko
added
[scope] bug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## sampler-engine-structure-280 #303 +/- ##
================================================================
+ Coverage 83.74% 83.82% +0.07%
================================================================
Files 68 68
Lines 5347 5348 +1
================================================================
+ Hits 4478 4483 +5
+ Misses 869 865 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
damskii9992
left a comment
damskii9992
left a comment
There was a problem hiding this comment.
Just a small comment and a question. Otherwise LGTM 👍
| effective_method = method if method is not None else self._method | ||
| if tolerance is not None: | ||
| if method in [None, 'least_squares', 'leastsq']: | ||
| if effective_method in [None, 'least_squares', 'leastsq']: |
There was a problem hiding this comment.
You can remove None from the list here, effective_method can never be None :)
There was a problem hiding this comment.
errr... the power of ctr-c/ctrl-v is strong
| assert item1.error == pytest.approx(0, abs=2.1e-1) | ||
| # Methods without error bars (e.g. lmfit's gradient-free powell/cobyla) | ||
| # report error as None rather than a fake 0.0. | ||
| assert item1.error is None or item1.error == pytest.approx(0, abs=2.1e-1) |
There was a problem hiding this comment.
Are there anywhere this is used where this new assert is now too loose? Just asking, haven't checked myself.
There was a problem hiding this comment.
for powell/cobyla the old assert was only confirming a hard-coded fake 0.0.
This was too loose in that nothing pinned which methods may return None.
Now each call site declares it, so any future potential changes to None in leastsq/Bumps/DFO will fail the test.
There was a problem hiding this comment.
I modified the test to explicitly include the new behaviour
2 participants
Parameter error handling improvements:
errorattribute set toNoneinstead of0.0, making it clear when uncertainty is genuinely unknown rather than zero.Tolerance keyword selection:
_get_fit_kwsnow determines the effective minimization method before choosing the appropriate tolerance parameter, ensuring correct configuration for different backends.