-
Notifications
You must be signed in to change notification settings - Fork 6
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
Energy-dependent pull correction #363
Comments
I am not sure I grasp all the details. I somewhat understand the expected behaviour but I don't quite understand the current behaviour and why it is problematic. Maybe you can elaborate about the "neither of the two solutions is what we need". In general, in am in favour in tidying up the configuration dictionaries in any sensible way :) |
The problem lies in how the BaseAngularErrorModifier is called in the minimizer, namely the Now if you choose the The alternative method, where the Another thing we could do, is create the pull_dict separately without taking the llh_energy_dict as it is in order to accomodate for when power_law is used as the llh energy pdf. In any case, my main question is more on the conceptual side of things: do we need to weight the MC to get the (energy-dependent) pulls? If weighting isn't needed, then the problem is solved by itself since weight_mc() is the one that requires the gamma |
Thank you for the explanation, I better understand the context and use case now. My understanding of the pull correction is that it is evaluated by comparing the true directional reco error with the angular error estimator. In this sense, you need to weight the MC because, similarly to other cases reco vs true , the resulting PDFs will depend on the assumed spectrum (since the PSF is energy-dependent). I believe the reason the 1D pull correction requires a fixed gamma in the likelihood is that, indeed, the pull (and hence the likelihood) depends on the gamma. To consistently fit gamma, one would need to re-evaluate the pull at every point in the gamma space, which I believe is what the 2D pull feature is there for. So my question is: do you actually want to fit a free gamma parameter when your likelihood embeds a component evaluated with a fixed gamma? Are there specific reasons for not using a 2D correction? If the answer to these questions is actually yes, then I suppose |
The reason I chose the fixed-gamma pull correction is because I'm currently testing only the scenario of gamma = 2 for a subsample of catalog sources, so that I can have a direct comparison with the sensitivities I get with the KDEs (KDEs inherently account for what the pull correction does). I agree that in any other case where you're fitting the gamma the 2D energy-dependent correction is appropriate (eg the Nevertheless, if everyone else agrees that weighting the MC is the sensible thing to do, I would suggest that the
in order to be able to inject with different spectrum. This change would be applicable to the fixed-gamma, energy-dependent pull correction, the energy- and declination-dependent pull correction, as well as all the @JannisNe any comments? |
…gy pdf from inj_dict instead of the llh_dict, so the gamma can be provided. Fixes issue #363
* when creating BaseAngularErrorModifier in the minimizer, use the energy pdf from inj_dict instead of the llh_dict, so the gamma can be provided. Fixes issue #363 * change mode from "r" to "rb" when reading pull pickle in BaseMedianAngularErrorModifier * correct inheritance in DynamicMedianPullCorrector * deprecated getargspec, change to inspect.getfullargspec instead in create_spatial_cache of DynamicMedianPullCorrector * change floor_dict["season"]["mc_path"] to floor_dict["season"].mc_path in get_mc() to correctly load the mc path from season * change ts_type from "Standard" to "standard" in plot_background_ts_distribution() to avoid exception raised in fit_background_ts() * correct path names for the discovery potential plots * create separate energy pdf dict passed in BaseAngularErrorModifier. For both static & dynamic floors & pulls the epdf name is the same as in llh dict. For static case where gamma is required, it is chosen from inj dict * raise error if KDE llh and pull correction (other than "no_pull") are chosen * stick to the llh epdf when creating the BaseAngularErrorModifier in the minimizer. Raise an error instead when choosing static floor/pull but dont provide gamma in the llh epdf dict * change from dynamic to static cases for floors and pulls when raising the error in add_angular_error_modifier
Issue fixed by throwing an error when creating the |
When applying an energy-dependent pull correction (eg
median_1d
), you weight the MC to get the (weighted) median pull. If you are using thepower_law
energy pdf, you need to provide the gamma in thee_pdf_dict
which is then used in theweight_mc()
.As it is right now, the pull_dict is created by taking the
llh_dict["llh_energy_pdf"]
, in which you need to pass the gamma in addition to the energy_pdf_name when setting up the mh_dict so that the pulls can be calculated. Alternatively, you can use theinj_dict["injection_energy_pdf"]
when creating the pull_dict in theadd_angular_error_modifier()
Neither of the two solutions is what we need, so the question is do we need to weight the MC to get the pulls? If yes, then the pull_dict needs to be created in such a way that the gamma is provided, but without requiring that the energy pdf is the same for both llh and injection. Any ideas on this?
To Reproduce
Run any analysis with
llh_dict["pull_name"] = "median_1d"
Either pass the gamma in the llh_dict:
or change the
BaseAngularErrorModifier
as aboveExpected behavior
It will run, after some errors are fixed :P
Additional context
Errors to fix so it can run
floor_dict["season"].mc_path
open(self.pull_name, "rb")
The text was updated successfully, but these errors were encountered: