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

Create TSmap_freeNormalization_3ML.ipynb #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PbU-Jason
Copy link

put my first version of TS map on intro_cosipy

@israelmcmc
Copy link
Collaborator

Hey @PbU-Jason ! Thanks for working on this, and sorry it took me so long to look at it.

It looks great! I didn't see any issue with the calculation.

Next week, we can work on:

  • Moving this from this notebook to the cosipy library
  • Use a healpix map instead of a rectangular RA/Dec grid

Longer term, we can:

  • Move the TS map calculation to 3ML, so it works for any instruments, and we can do a joint localization
  • Free the index
  • Use a multi-resolution map, so we can sample the TS more finely around the best location.

@israelmcmc
Copy link
Collaborator

I'm just going to wait a little bit to merge this, I didn't have time to actually run it today. But this is what we'll be doing next week anyway!

@hiyoneda
Copy link

hiyoneda commented May 3, 2023

Just a minor comment. In my environment (Apple M1 Max), I got an error in the 12th cell. When I changed spectrum.K.value, then it disappeared.

# Set the normalization to 0, that is, background-only null-hypothesis
#spectrum.K.value = spectrum.K.min_value # min_value is a very small value, approx 0. However, it can't be exactly 0 for some internal numerical reason
spectrum.K.value = 1e-12 # min_value is a very small value, approx 0. However, it can't be exactly 0 for some internal numerical reason

# Maximum likelihood
like.fit(quiet=True)

# Compute the likelihood
log_like0 = cosi.get_log_like()

@GallegoSav
Copy link

GallegoSav commented May 3, 2023

Before merging you need to change after the fit :

# Set the normalization to 0, that is, background-only null-hypothesis
spectrum.K.value = spectrum.K.min_value # min_value is a very small value, approx 0. However, it can't be exactly 0 for some internal numerical reason
# 10⁻30 is to low
spectrum.K.value = 1e-10

# Maximum likelihood
like.fit(quiet=True)

# Compute the likelihood
log_like0 = cosi.get_log_like()

spectrum.K.value need to be 1e-10 otherwise you will have a migrad error

@GallegoSav
Copy link

GallegoSav commented May 5, 2023

In order to remove the gammapy dependance, you need to remove from gammapy.modeling.models import PowerLawSpectralModel in the cell 1 and

# Here we translate to gammapy. We will make the detector response
            # accept 3ML models and there will be no need for this
            if not isinstance(spectrum, Powerlaw):
               raise RuntimeError("Only PowerLaw supported for now")

            spectrum = PowerLawSpectralModel(
                            index = -spectrum.index.value,
                            amplitude = spectrum.K.value * spectrum.K.unit,
                            reference = spectrum.piv.value * spectrum.piv.unit)
            

in cell 2

@PbU-Jason
Copy link
Author

Received! Thanks for helping me find some bugs~
I will set spectrum.K.value to be 1e-10, and also remove the gammapy dependance.

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.

4 participants