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

Discrepancy in probability/chi^2 #51

Open
ryanlevy opened this issue May 11, 2021 · 3 comments
Open

Discrepancy in probability/chi^2 #51

ryanlevy opened this issue May 11, 2021 · 3 comments

Comments

@ryanlevy
Copy link
Collaborator

Reported by Manuel Zingl and Khaldoon Ghanem (thanks both!) there are some potential subtle problems with definitions. The code seems to use Q=chi^2 / Ndat - alpha*S in the minimization routine rather than the reported Q=1/2chi^2 - alpha*S

Current options:

  • Use the Ndat version of Q and fix Q and chi2 functions in the code
    • we should test how things will change under this
  • Remove the (re)scaling
    • this seems to reasonably match e.g. MEM for the Bryan method
@jpfleblanc
Copy link
Collaborator

So what you think is most correct. Would be nice to see the impact of this on the example test cases as a minimum.

@ryanlevy
Copy link
Collaborator Author

case1.pdf
case2.pdf
case3.pdf

Khaldoon graciously provided three test cases along with their outputs from MEM i.e. Jarrell Code.
to be more explicit:

  • Original Code is the current release
  • Patched (1/2chi^2) is Khaldoon's proposed patch which removes the extra 2/ndat on lines 89 and 99 of maxent_helper.cpp
  • Patched (chi^2/ndat) carries through the extra factor of 2/ndat to the objective function Q so its consistent everywhere (changing the Q function )

@egull
Copy link
Contributor

egull commented May 17, 2021

This looks remarkably clean. I'm ok with changing the convention.

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

3 participants