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

[WIP] Add density and cdf plot functions for random variables #19073

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Smit-create
Copy link
Member

References to other Issues or PRs

Related to the discussion in #18730, See 18730(comment)

Brief description of what is fixed or changed

Other comments

  • Finalize the API for plot functions in stats
  • Add functions to plot density and cdf
  • Add Documentation and tests

Release Notes

  • stats
    • Added density and cdf plotting functions

@sympy-bot
Copy link

Hi, I am the SymPy bot (v158). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Related to the discussion in #18730, See [18730(comment)](https://github.com/sympy/sympy/issues/18730#issuecomment-596183566)

#### Brief description of what is fixed or changed


#### Other comments
- [ ] Finalize the API for plot functions in stats
- [ ] Add functions to plot `density` and `cdf`
- [ ] Add Documentation and tests

#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* stats
   * Added `density` and `cdf` plotting functions
<!-- END RELEASE NOTES -->

@sylee957
Copy link
Member

sylee957 commented Apr 5, 2020

Does it support both continuous and discrete?

@Smit-create
Copy link
Member Author

Yes, it plots for discrete Rv, but I was wondering If we have something like scatter plot in matplotlib(for plotting for discrete), also, some argument like xlim(presently doesn't work good) which plots only on the required domain?

@sylee957
Copy link
Member

sylee957 commented Apr 5, 2020

I don't think that it gives meaningful result for discrete random variables, but gives a plain axis with nothing plotted.
I don't think that there is any API in sympy for scatter plot or stem plots, so someone should develop the plotting module, but I'm afraid that it can quickly grow out of scope in this PR.

@Abdullahjavednesar Abdullahjavednesar requested review from jksuom and removed request for ritesh99rakesh April 5, 2020 17:50
@Smit-create
Copy link
Member Author

I don't think that there is any API in sympy for scatter plot

I agree, plotting of discrete RV as continuous lines does not look good. I had expected to create some plots for them as here. But, Currently, I think creating such plots will require refining plotting module. Please share your thoughts on this @czgdp1807

@@ -575,7 +575,7 @@ class LineOver1DRangeSeries(Line2DBaseSeries):
def __init__(self, expr, var_start_end, **kwargs):
super(LineOver1DRangeSeries, self).__init__()
self.expr = sympify(expr)
self.label = str(self.expr)
self.label = kwargs.get('label', None) or str(self.expr)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sylee957 Should I make this change in a new PR? It involves adding label as an argument in the plot so that we could use them to denote when we use legend to name that line as in matplotlib. Currently, it by default considers label as str(self.expr) . I will look something like this.

Screenshot from 2020-04-06 11-23-00

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that labeling a plot manually can be a useful feature.
But you should add tests for ordinary plots to see how this works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the plotting module able to plot a set of functions in the same graph as shown above? Is the public API of plot capable of supporting such stuff?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is able to plot many functions in the same graph. Seeing this as a useful feature, I made a change in the plotting module. But, I think, I should open a new PR for this as it involves a change in public API of plot. Also, I think the current plot is unable to plot discrete graphs as we have scatter plot in matplotlib which is required for plotting discrete RV.

@czgdp1807
Copy link
Member

czgdp1807 commented Apr 6, 2020

This PR creates a wrapper for plotting module to make things easy. So probably, making changes to plotting module should be restricted here.

@czgdp1807
Copy link
Member

Any news on this?

@Smit-create
Copy link
Member Author

I was wondering how to proceed further without scatter plots which are necessary for both FRV and DRV

@czgdp1807
Copy link
Member

Can we use histograms for both FRVs, and DRVs? I remember, plotting was also meant for random matrices too, though I am not sure how should they be represented on graphs.

@sylee957
Copy link
Member

The problem of histogram is that setting the bin size is arbitrary, and again I don't think that it is the best way to display discrete PMF than stem because the visuals are significantly affected by the width of the bins.

@czgdp1807
Copy link
Member

May be, https://matplotlib.org/3.2.1/api/_as_gen/matplotlib.pyplot.scatter.html can be tried out for FRV and DRV. Is there any support for wrapper of the above API in plotting module?

@czgdp1807 czgdp1807 added the GSoC label May 5, 2020
@czgdp1807
Copy link
Member

Any news on this?

@Smit-create
Copy link
Member Author

Is there any support for wrapper of the above API in plotting module?

We currently don't have scatter plots available in plotting module. Also #19073 (comment)

@czgdp1807
Copy link
Member

What about using matplotlib API as suggested in #19073 (comment)
So, we should open a new issue for proposing inclusion of wrappers for scatter plots in plotting module. It's optional for you to work on that issue as it is out of scope of your GSoC project. Let's stall this PR until then if there isn't any better idea to plot FRVs and DRVs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants