Skip to content

Add MDP for maximum likelihood method#525

Open
eneights wants to merge 13 commits intocositools:developfrom
eneights:develop
Open

Add MDP for maximum likelihood method#525
eneights wants to merge 13 commits intocositools:developfrom
eneights:develop

Conversation

@eneights
Copy link
Contributor

I added a function to calculate the minimum detectable polarization for the maximum likelihood method, and added the calculation to the example notebook. This runs quite slowly, and should probably be parallelized. The notebook only runs 100 iterations for now, but in reality we will want something like ~10,000. It also currently can only handle a single source and background component.

@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.89%. Comparing base (0d7d6d6) to head (7ebddff).
⚠️ Report is 10 commits behind head on develop.

Files with missing lines Patch % Lines
cosipy/sensitivity/mdp.py 98.03% 1 Missing ⚠️
Files with missing lines Coverage Δ
cosipy/polarization_fitting/polarization_asad.py 85.71% <ø> (-0.07%) ⬇️
cosipy/sensitivity/mdp.py 98.03% <98.03%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Israel Martinez <imc@umd.edu>
@israelmcmc
Copy link
Collaborator

Thanks @eneights.

I added the __init__.py file to the new sensitivity folder is recognized as a module. It needs one even if it's empty. I also synced your branch with develop.

Issues/requests/comments:

  • Since now sensivitiy is recognized as a module, codecov is correctly catching that you haven't written unit test for it yet.

  • The tutorial is complaining the compute_mdp was not imported. For reference, you can check that the tutorial works in a clean run with

~/software/cosipy/docs/tutorials/run_tutorials.py ~/software/cosipy/docs/tutorials/run_tutorials.yml -o tutorials --wasabi_mirror ~/cosi/data/wasabi/cosi-pipelines-public/ --tutorial polarization_ml
  • Have you looked into the fail fits? 20% failure is like a large fraction, considering that the MDP is based on the ~99%, if those simulations that would have had a large fake PD are more likely to fail, then the MDP could be underestimated.

  • It seems to me that you can change (spectrum, model, source_direction) for a single input e.g. source that received a PointSource. You can get all of the information from these 3 parameters from there and at the same time makes sure you only have a single source.

  • Similarly, you don't need fix_bkg, since you can pass this information through bkg_parameter e.g.

Parameter('total_bkg',  # background parameter
                                            0.0016,  # initial value of parameter
                                            min_value=0,  # minimum value of parameter
                                            max_value=100,  # maximum value of parameter
                                            delta=0.05,  # initial step used by fitting engine
                                            unit = u.Hz,
                                            free = False)

A note for later: I think that eventually this function need to be split in two: a generic one that takes (source: PointSource, background:BackgroundInterface, response:ThreeMLSourceResponseInterface), and a helper function that builds (source, background,response) from the specific implementation you currently have. However, this PR made me realize that I first need to change slightly the definitions of some interfaces to make this possible.

@eneights
Copy link
Contributor Author

Thanks, @israelmcmc! I'm not sure what to make of the fits failing and it does make me worry the MDP is being underestimated. But I'm not sure what to look into to see why they're failing, since they're all unpolarized and the only difference between them is the Poisson fluctuations.

About changing (spectrum, model, source_direction) for a single input, I had tried quickly before to only have the model as an input since this contains the spectrum and source_direction, but I couldn't figure out how to get the spectrum object out instead of just the dictionary of parameters. I'll look into it more a little later.

@eneights
Copy link
Contributor Author

It's updated now so the user only has to input the model and not the spectrum and position in addition

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.

2 participants