Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct init_vals #821

Merged
merged 3 commits into from
Nov 8, 2022
Merged

Correct init_vals #821

merged 3 commits into from
Nov 8, 2022

Conversation

newville
Copy link
Member

Description

This ensures that result.init_vals is the "user" / "external" values, and adds a new attribute result._init_vals_internal to store the initial "internal" values which are needed as initial values.

This addresses #820

When running tests, I saw some deprecations and addressed some of those, but it seems there are other deprecation warnings that might need more work.

Type of Changes
  • Bug fix
  • New feature
  • Refactoring / maintenance
  • Documentation / examples
Tested on

import sys, lmfit, numpy, scipy, asteval, uncertainties
print('Python: {}\n\nlmfit: {}, scipy: {}, numpy: {}, asteval: {}, uncertainties: {}'
... .format(sys.version, lmfit.version, scipy.version, numpy.version,
... asteval.version, uncertainties.version))
Python: 3.9.13 | packaged by conda-forge | (main, May 27 2022, 17:00:52)
[Clang 13.0.1 ]

lmfit: 1.0.3.post69+g99952a2.d20221031, scipy: 1.9.0, numpy: 1.23.2, asteval: 0.9.27.post12+ge349473, uncertainties: 3.1.7

Verification

Have you

  • included docstrings that follow PEP 257?
  • referenced existing Issue and/or provided relevant link to mailing list?
  • verified that existing tests pass locally?
  • verified that the documentation builds locally?
  • squashed/minimized your commits and written descriptive commit messages?
  • added or updated existing tests to cover the changes?
  • updated the documentation and/or added an entry to the release notes (doc/whatsnew.rst)?
  • added an example?

closes #820

@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Merging #821 (1a8f035) into master (4173760) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #821      +/-   ##
==========================================
+ Coverage   93.71%   93.73%   +0.01%     
==========================================
  Files           9        9              
  Lines        3487     3498      +11     
==========================================
+ Hits         3268     3279      +11     
  Misses        219      219              
Impacted Files Coverage Δ
lmfit/minimizer.py 90.70% <100.00%> (+0.02%) ⬆️
lmfit/lineshapes.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@reneeotten
Copy link
Contributor

@newville we would need to look in which SciPy version the keyword(s) are deprecated. It it's still present in our lowers supported version we will need to use a different approach (as we've done for other minimizers as well). Should we bump supported NumPy/SciPy versions for the next release?

I'll take a look at the other code changes later today.

@newville
Copy link
Member Author

newville commented Nov 1, 2022

@reneeotten I see messages about deprecations of the dreaded topic of maxiter in favor of maxfun within scipy (scheduled for scipy 1.11) when running tests locally. In a first pass, I fixed one of those, but other changes that I tried in our minimizer.py was causing several of our tests to fail.

I think that issue (probably should have been raised separately from this issue on init_vals) probably needs more attention. Will raise a separate issue.

@newville
Copy link
Member Author

newville commented Nov 5, 2022

I should have mentioned that the changes at 1a8f035 were to suppress warnings when testing locally with Python 3.11

@newville
Copy link
Member Author

newville commented Nov 8, 2022

merging this...

@newville newville merged commit 60eedb0 into master Nov 8, 2022
@reneeotten reneeotten deleted the correct_init_vals branch November 19, 2022 17:57
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.

Incorrect value in MinimizerResult.init_values
2 participants