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

General restructuring of the SED for their usage as fittable models #52

Merged
merged 32 commits into from
Nov 16, 2020

Conversation

cosimoNigro
Copy link
Owner

@cosimoNigro cosimoNigro commented Nov 16, 2020

This PR implements many internal changes aimed at creating functions suited to be passed to a fitting procedure.

  1. I introduced staticmethods for each of the radiative processes such that the SED can be computed changing all the parameters together.
    The problem was this: if we wanted to fit an external Compton SED with the previous version of the code,
    half of the parameters were contained in the Blob instance, the other half in the target instance.
    Therefore a fitting procedure would have needed to access the objects individually, change them, and redefine the ExternalCompton class with them.
    Now everything can be done within a single function containing all the parameters of the model, e.g.
Synchrotron.evaluate_sed_flux(
    nu, z, d_L, delta_D, B, R_b, LogParabola, k_e, p, q, gamma_0, gamma_min, gamma_max
)

for the Synchrotron radiation

SynchrotronSelfCompton.evaluate_sed_flux(
    nu, z, d_L, delta_D, B, R_b, LogParabola, k_e, p, q, gamma_0, gamma_min, gamma_max
)

for the SSC.
You can find in experiments/basic/sherpa_fitting_ssc_trial.ipynb an example of how I plugged these new functions in sherpa to create a fittable model. The fit is not decent but I did not play that much with the parameters!
I just wanted to be sure that the agnpy model could be coupled to sherpa.

Of course the high-level function sed_flux(nu) has remained the same.

  1. I introduced a new integration procedure half-copied from naima.
    It is an implementation of the trapezoidal rule in log-space. Each slice of the function is interpolated with a power-law rather than with a straight line. It is slightly more precise of the trapezoidal rule in linear space. Both for the spectra and for the radiative processes one can choose whether to use the trapezoidal rule in log-log space or the normal one when integrating through the integrator argument.
    Here a comparison of the SED for EC on Disk computed with both the integration methods.
    ec_disk_comparison_integration_methods

  2. I added some functionalities in utils that we often use (conversion, geometric computation, plotting routines).

  3. The only major change in the API is this: it is not needed anymore to pass the Synchrotron object to the SynchrotronSelfCompton class. It was just redundant.
    SynchrotronSelfCompton(blob, synchro) is now just SynchrotronSelfCompton(blob)
    I updated the docs accordingly.

I marked this modifications as 0.0.7.9, If anyone wants to try it, please do it.
We can make the 0.0.8 release in few days.

…ntegration

General restructuring of the SED for their usage as fittable models (Sourcery refactored)
@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #52 (4081bcd) into master (179973a) will increase coverage by 3.08%.
The diff coverage is 94.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   90.90%   93.99%   +3.08%     
==========================================
  Files          16       30      +14     
  Lines        1462     1681     +219     
==========================================
+ Hits         1329     1580     +251     
+ Misses        133      101      -32     
Flag Coverage Δ
unittests 93.99% <94.27%> (+3.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
agnpy/constraints/spectral_constraints.py 68.75% <ø> (ø)
setup.py 0.00% <ø> (ø)
agnpy/utils/plot.py 47.05% <47.05%> (ø)
agnpy/emission_regions/blob.py 88.67% <80.00%> (ø)
agnpy/targets/targets.py 79.66% <84.84%> (ø)
agnpy/compton/external_compton.py 87.50% <87.50%> (ø)
agnpy/compton/synchrotron_self_compton.py 87.80% <87.80%> (ø)
agnpy/synchrotron/synchrotron.py 94.18% <94.18%> (ø)
agnpy/spectra/spectra.py 97.39% <95.45%> (ø)
agnpy/__init__.py 100.00% <100.00%> (ø)
... and 39 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 179973a...4081bcd. Read the comment docs.

@cosimoNigro cosimoNigro merged commit 5d69ed1 into master Nov 16, 2020
@cosimoNigro
Copy link
Owner Author

@jsitarek I apologise if this breaks the changes you were doing to check the absorption.
Notice now I created a submodule for each component (emitters and radiative processes).
I thought I could make this changes faster but then the preparation for the MAGIC meeting and the ADASS got me stuck.

Let me know what do you think of the handling of the fitting procedure, as you can see in the notebook I mentioned above it takes few lines to define a sherpa model.
We can open a new issue to discuss this.

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.

1 participant