-
Notifications
You must be signed in to change notification settings - Fork 2
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
Test scripts with SED fit with gammapy-0.19 #18
Comments
Comments after the call of Friday 26. As I said, gammapy FluxPoint do not accept entries with the same energy now. For this, my first try was to remove the duplicated entries with
I reran the sherpa fit for PKS 1510-089, removing the duplicated entries as mentioned above. The first plot is the result with gamampy the second with sherpa. Both are different from the paper but more important, I do not have the same results :( More to come soon |
Thanks a lot @davidsanchez, I wrote in the Gammapy slack to get some support on this, I CC you. |
@cosimoNigro . I saw your message. thanks. I reply to it. I found that using the sherpa script and only sorting the data by increasing energy gives the strange results reported above. I am looking into that |
Just for there record: the new recommended way to handle multiple flux points in Gammapy is to just keep them separate with multiple We found that updating to |
Thanks a lot for commenting here Axel, I get the advantages of the new approach. The only solution is to contact the author of the paper and hope he still has the data separated by instrument. Otherwise the only workaround is to do a re-binning of the flux points, with the flux in each bin being the weighted average of all the points. |
Alternatively we can also just release the agnpy paper with gammapy-0.18.2, we have a conda environment and a docker container. The problem has to be fixed though in the notebooks in the docs. |
Ok I wrote to David Paneque to see if he still has the Mrk421 measurements separated by instruments. @jsitarek would you have the PKS1510-089 2015 data with the flux measurements by different instruments distinguishable? |
Hi, in my notes I have them all put together, but I will investigate if I can recover the original ones (the problem seems to be two spectra from UVOT). Alternatively one could split them just into higher and lower values.
|
Hi, I was able to dig out the original script and get the data, I was wrong, it was not UVOT that was doubled, but some optical data, and they are mostly coming from the same instrument but different pointings, in particular TCS is messy, because I have 6 points in one night and 3 different wavelenghts, but they are not in two series, but kind of mixed. I marked each of them separately, but honestly one could simply divide them randomly in two groups just to bypass the gammapy issue. Here is the divided list (note the units issue mentioned above, I also included the missing point) POINTS FOR MJD=57164-57166 |
@davidsanchez if you open a draft PR with your modifications I can devote some time to help you. |
@cosimoNigro |
I can see the change in Gammapy creates a bit of inconvenience on the I/O side and data bookkeeping here. I think a good solution would be to make use of Astropy's table grouping. So you could keep all the flux points in a single file, but add an "instrument" or "tag" column by hand. Then in the code you can use something along the lines of: datasets = Datasets()
table = Table.read("flux-points-all.ecsv")
table = table.group_by("instrument")
for group in table.groups:
data = FluxPoints.from_table(group, sed_type="e2dnde", format="gadf-sed")
dataset = FluxPointsDataset(data=data, name=group["instrument"][0])
datasets.append(dataset)
datasets.models = SkyModel() |
Hi |
@davidsanchez But removing "duplicated" entries should change the result, no? As far as I can see there are some energies with multiple measurements available. Removing those should definitely change the result, as it affects the likelihood that is computed. |
@adonath indeed. I the thread I mentioned that I am now sorting the data only and shifting the energy by epsilon. Sorry this was not clear enough; sed_table = Table.read(sed_path)
mask = np.zeros_like(sed_table["e_ref"], dtype=bool)
mask[np.unique(sed_table["e_ref"], return_index=True)[1]] = True
mask = ~mask
sed_table["e_ref"][mask.tolist()] +=0.0000001
sed_table.sort(keys="e_ref")# sorting data wrt e_ref Also, using sherpa and only sorting the data by energy, I get a different results. This not related to gammapy then |
Hey @adonath, thanks a lot for helping us. Here a minimal example, a
This is just one of the examples of the single-valued from astropy.table import Table
from gammapy.estimators import FluxPoints
table = Table.read("dummy_table.ecsv")
data = FluxPoints.from_table(table, sed_type="e2dnde", format="gadf-sed") this returns:
I guess you cannot define the MapAxis with a center without edges... |
Thanks for putting in the work to update and providing feedback as well @cosimoNigro! I just checked our issue tracker and re-opened gammapy/gammapy#1952. It was a known issue, but at that time we only implemented the exact check that you see above and did not decide to actually support This also points to a possible workaround. Gammapy gives the from gammapy.estimators import FluxPoints
from astropy.table import Table
from io import BytesIO
yaml = b"""
# %ECSV 0.9
# ---
# datatype:
# - {name: e_ref, unit: eV, datatype: float64}
# - {name: e_min, unit: eV, datatype: float64}
# - {name: e_max, unit: eV, datatype: float64}
# - {name: e2dnde, unit: erg / (cm2 s), datatype: float64}
# - {name: e2dnde_errn, unit: erg / (cm2 s), datatype: float64}
# - {name: e2dnde_errp, unit: erg / (cm2 s), datatype: float64}
# - {name: instrument, datatype: string}
e_ref e_min e_max e2dnde e2dnde_errn e2dnde_errp instrument
0.000153032 0.00015150168 0.00015457777777777776 6.99043e-13 3.2562e-14 3.2562e-14 Metsahovi
"""
table = Table.read(BytesIO(yaml), format="ascii.ecsv")
fp = FluxPoints.from_table(table, format="gadf-sed", sed_type="e2dnde")
print(fp.energy_ref) The I think we can resolve gammapy/gammapy#1952 by adding the support for |
A new release of gammapy will be announced tomorrow.
We should try to update the scripts with the SED fit to this latest version.
With 0.1.8.2 there is actually a conflict complicating the reproducibility: gammapy 0.18.2 works with iminuit < 2 while jetset works with higher versions. Not that we are using any fitting routine of jetset, but installing it will automatically install a newest version of iminuit. The conflict should be resolved by gammapy 0.19 that upgraded its iminuit dependency to the latest version.
The text was updated successfully, but these errors were encountered: