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

Fix weight handling in fitting #511

Merged
merged 5 commits into from
Sep 17, 2019
Merged

Conversation

nmearl
Copy link
Contributor

@nmearl nmearl commented Sep 12, 2019

Addresses #505.

@nmearl nmearl requested a review from eteq September 12, 2019 19:19
@nmearl
Copy link
Contributor Author

nmearl commented Sep 12, 2019

Reposting conversation from previously incorrect PR:

@nmearl:

@eteq Strangely, there's a difference when using the weights explicitly by passing in weights=1 / unc, and having them be implicit weights='unc' (where 'unc' means do 1 / spec.uncertainty.array). You can see the difference in value in the test...

@hcferguson:

@nmearl As I mentioned on some previous ticket....usually one wants the "weights" to end up being (uncertainty**-2). Perhaps it's doing that internally when you pass it weights='unc'?

It's sort of semantics...if chisq1 = sum(w * (predicted-model)**2) then you want w=uncertainty**-2. If chisq2 = sum(w**-2 * (predicted-model)**2), then you want w=1/unc. My impression is that chisq1 is more common practice when talking about the relation between weights and uncertainties.

@nmearl:

@hcferguson Ah, that's correct. The astropy fitters (e.g. the LevMarLSQFitter implements the chisq1 approach when using weights -- I'll fix the unc version accordingly.

@nmearl nmearl added this to the v0.6 milestone Sep 13, 2019
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.

1 participant