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

Suggestion to remove standard_llh #374

Open
pernicet opened this issue May 21, 2024 · 5 comments
Open

Suggestion to remove standard_llh #374

pernicet opened this issue May 21, 2024 · 5 comments

Comments

@pernicet
Copy link

I have been using Jannis Flaires catalogue to calculate its sensitivity:

  • dataset: ps_v004_p02
  • minimisation handler: fixed_weights
  • weight: bolometric fluence
  • 528 sources
  • Gamma = 2

The two likelihoods tested are:

  1. standard_llh
  2. standard_matrix

The n_s bias plots look like:

standard_llh
bias_n_s-25.pdf
standard_matrix
bias_n_s-26.pdf

Similar behaviour appears in the gamma bias plots. I also tested sub catalogues of Flaires with 100, 200, 300 and 400 sources and the behaviour it's the same: a huge deviation is observed when using the standard_llh (even with fewer sources), which it seems to disappear when using standard_matrix. Do we know why? Should we perhaps reconsider using standard_llh at all?

@JannisNe
Copy link
Collaborator

Thanks, yes, I think we should make standard_llh unusable. Something like #375 should work.

@pernicet
Copy link
Author

pernicet commented Jun 6, 2024

The same behaviour is observed when I use standard_kde_enabled when using KDEs with the northern tracks sample:
bias_n_s-45.pdf
Maybe we should reconsider using this one as well.

@mlincett
Copy link
Collaborator

mlincett commented Jun 6, 2024

Thanks, yes, I think we should make standard_llh unusable. Something like #375 should work.

Does standard_matrix have the same performance of standard_llh for a single pointsource? I would definitely drop standard_llh for any stacking because it's more likely to screw up than actually work, but I still wonder if it provides any performance benefit for a single source.

@JannisNe
Copy link
Collaborator

Do you mean performance in terms of runtime? I have never tested it, so I don't have numbers, but I feel that it will not make a big difference.

@sathanas31
Copy link
Contributor

In principle the standard_kde_enabled can be deleted altogether on top of removing it from compatible_llh since it's not even tested. Also, for future reference, we could merge StandardOverlappingLLH with StandardLLH since the only difference between the 2 are the create_kwargs & calculate_test_statistic methods. This is too big of a change, and the existing tests need to be modified accordingly, so I second Jannis' solution for now.

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

4 participants