Closed juanep97 closed 2 years ago
Merging #114 (087052f) into master (86a28d4) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #114 +/- ##
=======================================
Coverage 96.99% 96.99%
=======================================
Files 30 30
Lines 2233 2233
=======================================
Hits 2166 2166
Misses 67 67
Flag | Coverage Δ | |
---|---|---|
unittests | 96.99% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 86a28d4...087052f. Read the comment docs.
Dear @jepcor97,
apologies for replying so late and thanks a lot for the effort of updating the notebooks.
I do not like the idea of deleting data points with repeated energies in the dataset.
Now, we had this discussion with @adonath while preparing the agnpy_paper
.
If an additional info is available in the table (e.g. a string with the instrument name), Axel showed how to separate the flux points belonging to each instrument and then define a single FluxPointsDataset
for each of them.
https://github.com/cosimoNigro/agnpy_paper/issues/18#issuecomment-982698805
This would solve the issue for the PKS1510-089 dataset, for which, thanks to Julian, we solved the problem of provenance.
There was a problem with single-bin flux measurement, but this will be fixed as soon as https://github.com/gammapy/gammapy/pull/3807 is merged in gammapy-master
.
I contacted the authors of the Mrk421 2011 to ask if they had the MWL SED ordered by instrument, but did not receive any answer so far. I suggest we make an other attempt - I am writing to them now, and if we do not manage, then let us simply change the dataset! I am sure we can find another MWL SED, even from the same Mrk421.
As I said I do not like the idea of canceling measurements just because we cannot make flux points with them.
Also Andrea is using the same dataset in jetset
, though he re-bins the flux points. I think it would be nice to keep the consistency between the two software examples.
Thanks for your work Juan.
Since that dataset is "iconic" we can also consider keeping it and simply show an example of fit with sherpa
using it.
And choose another BL Lac to fit with Gammapy.
We can differentiate the sources fitted with Gammapy
and sherpa
.
Hi @cosimoNigro, You are right, it would be much better to use a dataset with provenance, so we can separate them like in that link you sent. In fact, that would also automatically suply a tutorial using a composed dataset, which would illustrate the need of flattening that array I told you about through slack. When we have the new dataset, I can try to update the notebooks to it. The remaning modifications of the tutorials, some because of matplotlib and some because of gammapy dev, are still necessary, I think. Let's wait then. Thank you very much.
@jepcor97, I added Gammapy
and sherpa
wrappers in a parallel PR #119.
This PR is superseded by it.
Thanks anyhow for your work.
This PR modifies the notebooks so they are able to run with gammapy dev version.
Changes in this PR:
Mrk421_2011.ecsv
andPKS1510-089_2015b.ecsv
removing points with repeeated energy since they could not be loaded by gammapy; as a workaround until this possibility is implemented.FluxPoints
instances don't have an accesible.data
attribute anymore, and it gave errors.kwargs
passed to plot) since the names had changed and they were not anymore compatible.