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

Improve error messages when users try to plot multi-dimensional data #849

Closed
wants to merge 2 commits into from

Conversation

parejkoj
Copy link

@parejkoj parejkoj commented Mar 8, 2023

Description

When I tried to plot a model that is inherently 2-dimensional (e.g. an image, with independent variable shape (2,N,N)), I got the following matplotlib exception:

ValueError: x and y must have same first dimension, but have shapes (2, 10, 10) and (10, 10)

There is an existing check on whether there is more than one independent variable, but that doesn't help if there's e.g. an x/y grid passed in as a single variable (as is suggested in some of the docs). This branch gives one way to fix that, but I don't know if it's generic enough. I also changed the "print message" to "raise exception", to make it much more explicit what's going wrong.

On a separate commit, I changed any(np.iscomplex(z)) to np.iscomplex(z).any(), as the former doesn't work on data with a dimension (len(shape)) greater than 1.

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

Python: 3.10.9 | packaged by conda-forge | (main, Feb 2 2023, 20:20:04) [GCC 11.3.0]
lmfit: 0.0.post2673+g729491d, scipy: 1.10.0, numpy: 1.21.6, asteval: 0.9.28, uncertainties: 3.1.7

Verification

Have you

  • [x ] included docstrings that follow PEP 257?
  • referenced existing Issue and/or provided relevant link to mailing list?
  • [x ] verified that existing tests pass locally?
  • [x ] verified that the documentation builds locally?
    -- The only error I got was in a doc file I didn't touch: examples/index.rst.
  • [x ] squashed/minimized your commits and written descriptive commit messages?
  • [x ] added or updated existing tests to cover the changes?
  • updated the documentation and/or added an entry to the release notes (doc/whatsnew.rst)?
    -- Not sure what to add to whatsnew yet.
  • added an example?

Python's built-in `any` raises "ValueError: The truth value of an array
with more than one element is ambiguous. Use a.any() or a.all()" if the
array is multi-dimensional.
Raise an exception instead of printing a message, to make it very clear
that this is not possible.
Add test that this fails for one style of multi-dimensional data.
@parejkoj parejkoj force-pushed the 2d_fit_plot_errors branch from 66cb107 to 00eddc0 Compare March 8, 2023 22:47
@newville
Copy link
Member

newville commented Mar 8, 2023

@parejkoj It is important to discuss the problem you are seeing or raise an issue describing the problem before you propose a solution.

Please be clear about what you want: a 2D image of data and fit, or a fit projected to 1 axis?
Is this something you need right now or something that you believe we should support for everyone else?

@parejkoj
Copy link
Author

parejkoj commented Mar 8, 2023

Sorry: my branch here is a proposal to make the error message more useful. When I saw plot, I thought "oh, that might help me debug my failing fits" without realizing that I couldn't actually plot 2d data. I don't expect lmfit to support plotting multi-dimensional data (that's hard!), but the error message should at least be clear about what's wrong. It took me a while to figure out what the matplotlib exception was telling me.

I'll make the PR title more clear.

@parejkoj parejkoj changed the title 2d fit plot errors Improve error messages when users try to plot multi-dimensional data Mar 8, 2023
@newville
Copy link
Member

newville commented Mar 8, 2023

Improving the title of the PR is still proposing a solution to an essentially undocumented issue. Please start by stating the problem or even better, ask why something did not work as you expected.

@parejkoj
Copy link
Author

parejkoj commented Mar 8, 2023

I've updated first sentence of the description.

The issue is that if you try to call plot with an image-like model (independent variable shape (2,N,N), typically a numpy.meshgrid), you get a confusing matplotlib error, because the existing check in plot is just a check that there is only one element of independent_vars in the model, not that that one element might be multi-dimensional.

I can't find them right now, but there are some recommendations for handling 2d models and data lmfit in this manner (as a single 2xN variable that gets unrolled in the model internals).

@newville
Copy link
Member

newville commented Mar 9, 2023

@parejkoj Please begin with a discussion on the mailing list or GitHub Discussion that clearly illustrates the problem you are seeing. Include a minimal example of code that shows the problem, and describe why you think it is a problem that needs to be solved within lmfit.

I will admit to being somewhat skeptical of adding checks of data types or shapes of independent variables (witness your issue #850 -- adding such checks is error-prone even if the error is only "completeness").

I also admit to being somewhat skeptical of making changes related to plotting.

@parejkoj
Copy link
Author

parejkoj commented Mar 9, 2023

This branch added a test demonstrating the problem. I don't have time for further discussion on this, and since you don't think it's worth it, you can just close this PR.

@parejkoj parejkoj closed this Mar 9, 2023
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