Skip to content

added akaike and bayesian information criterion and documentation - #3700

Closed
astronomeralex wants to merge 2 commits into
astropy:masterfrom
astronomeralex:add-akaike-and-bayesian-information-criterion
Closed

astronomeralex wants to merge 2 commits into
astropy:masterfrom
astronomeralex:add-akaike-and-bayesian-information-criterion

Conversation

@astronomeralex

Copy link
Copy Markdown
Contributor

I've added two common information criterion in astro, with an example for each and a reference to a paper where they're discussed.

@crawfordsm

Copy link
Copy Markdown
Member

Hi @astronomeralex thanks for contributing this!

Can I ask you to add some tests as well? It would be helpful to at least add one test for each tasks that test whether the correct value is returned for a given set of values.

It also would be helpful to test that the code properly handles if the code can correctly handle the different types of the variables passed to it (single floats or arrays ).

And it would appear that the build is currently failing on

 assert num_samples - num_params - 1 > 0

where it isn't sure if num_params is an array or float. This would be a good example of something that would need a test.

@astronomeralex

Copy link
Copy Markdown
Contributor Author

@crawfordsm right, yeah, num_params can be an array or a float. i'll fix that and add some tests today.

@astronomeralex

Copy link
Copy Markdown
Contributor Author

@crawfordsm -- after fighting tests and typos, i think this is ready to merge?

@astronomeralex
astronomeralex force-pushed the add-akaike-and-bayesian-information-criterion branch from 203c869 to e66763d Compare April 24, 2015 11:55

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not to be paranoid, but where to the numbers 24.70588 ... and 17.5 come from? Do you know that this example is true (e.g. from a trustworthy reference) or did you calculate them with your new code, in other words, were they calculated assuming the code is right? (In that case, the test still has some value because it prevents bugs that could will be introduced through code changes in the future, but obviously it would be best to test a "known good" example to make sure that the current implementation is right.)

@embray

embray commented May 4, 2015

Copy link
Copy Markdown
Member

I wonder about how this could be added to astropy.modeling. For example, could we add this to the fit_info attached to some fitters? Would these even be useful for any of the existing fitters? I don't know the math here so I don't know the answers to these questions.

@astrofrog

Copy link
Copy Markdown
Member

@astronomeralex - could you implement the fixes suggested by @hamogu, @adrn, and @embray?

@embray - I agree we should think about this, but this should not hold back this PR.

@cdeil

cdeil commented May 22, 2015

Copy link
Copy Markdown
Member

Is this the same as the aic and bic functions in statsmodels.tools.eval_measures (see here)?

If yes, I'm -1 to re-implement them in astropy.stats ... instead, point the astronomers to statsmodels from the astropy.stats docs here.

@embray

embray commented Jul 7, 2015

Copy link
Copy Markdown
Member

@astronomeralex Any time to update this PR. Failing that, is there anyone who can be assigned to it to ferry it through?

@crawfordsm

Copy link
Copy Markdown
Member

@astronomeralex just pinging you on this to see if you have some time to finalize this?

@mirca

mirca commented Mar 12, 2016

Copy link
Copy Markdown
Member

If @astronomeralex does not have time to finish this, and if you guys think this is still relevant whatsoever, I would like to be assigned to finish it up.

@crawfordsm

Copy link
Copy Markdown
Member

@mirca if you want to work on this next, that would be helpful.

@crawfordsm

Copy link
Copy Markdown
Member

Closing this after #4716 was merged.

@crawfordsm crawfordsm closed this Apr 17, 2016
@astronomeralex
astronomeralex deleted the add-akaike-and-bayesian-information-criterion branch February 24, 2017 21:25
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.

8 participants