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

Add BinningAlgorithm algo type #166

Open
oschulz opened this issue Jul 29, 2020 · 4 comments
Open

Add BinningAlgorithm algo type #166

oschulz opened this issue Jul 29, 2020 · 4 comments

Comments

@oschulz
Copy link
Member

oschulz commented Jul 29, 2020

We currently handle choice of default binning differently in bat_marginalmode and bat_marginalize. We should add BinningAlgorithm as a new super-type for auto-binning algorithms (and also offer user-specified fixed binning).

@Cornelius-G
Copy link
Collaborator

I think I can take a look at this, maybe together with @VasylHafych, to harmonize our efforts from marginal mode and plotting in this new type.

@oschulz
Copy link
Member Author

oschulz commented Jul 30, 2020

Thanks! We can split _auto_binning_nbins among separate subtypes of AbstractBinningAlgorithm, and have a FixedBinning too or so. Of course we should allow AbstractArrays and NamedTuples of binning algos (like you already implemented for plotting) so the user can specify binning in a per-parameter fashion.

@Cornelius-G
Copy link
Collaborator

Ok, so just to make sure I understand what you propose:

We will add a AbstractBinningAlgorithm as a supertype for the different automatic binnings, like Freedman–Diaconis and so on.
These types could then be passed to plot and bat_marginalize as a keyword argument like plot(samples, :a, bins=FD()) and would trigger the right version of _auto_binning_nbins.
But how exactly should we handle user-defined binning, like fixed number of bins or bin edges. Should it be possible to pass them directly as Ints and Ranges, like currently in plotting: plot(samples, bins=(200, -1:.01:1))?
Or would these need to be put into a subtype like FixedBinning, so that plot(samples, bins = FixedBinning(200, -1:0.1:1)) ?

@oschulz
Copy link
Member Author

oschulz commented Jul 30, 2020

Yes, that's the idea. FixedBinning would indeed be a subtype of AbstractBinningAlgorithm.

To make life more comfortable for the users, we should define Base.conv(::Type{AbstractBinningAlgorithm}, x) for symbols (e.g. :fd gets converted to Freedman–Diaconis()), scalars (like 500 means 500 bins, ends of range auto-adjusted) and ranges (0:0.2:1000 will be converted to an instance of FixedBinning).

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

No branches or pull requests

2 participants