-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix the black body SEDs of the Disk and the Dust Torus #92
Conversation
Codecov Report
@@ Coverage Diff @@
## master #92 +/- ##
==========================================
+ Coverage 94.46% 95.48% +1.02%
==========================================
Files 30 30
Lines 2094 2127 +33
==========================================
+ Hits 1978 2031 +53
+ Misses 116 96 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for a delay on checking this code, I spotted an issue with lack of observation angle dependence on the disk luminosity.
agnpy/targets/targets.py
Outdated
@@ -343,17 +336,50 @@ def phi_disk_mu(self, mu, r_tilde): | |||
def phi_disk(self, R_tilde): | |||
return 1 - np.sqrt(self.R_in_tilde / R_tilde) | |||
|
|||
def T(self, R_tilde): | |||
def T(self, R): | |||
r"""temperature of the disk at radius :math:`\tilde{R}`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\tilde(R) ==> R in docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected
sed_disk = SSDisk.evaluate_multi_T_bb_sed(nu, z, M_BH, m_dot, R_in, R_out, d_L) | ||
# renormalise | ||
L = (np.trapz(sed_disk / nu, nu) * 4 * np.pi * d_L ** 2).to("erg s-1") | ||
norm = (L_disk / L).to_value("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a small issue here, for the renormalization you assume that the emission is isotropic (hence 4 pi), but since it is a black body flat surface (for a thin disk approximation) it should emit ~ cos (observation angle)
if you integrate it over the sphere I think what you are missing in norm is an additional factor: 2* cos (observation_angle).
you would also need to update the test accordingly.
Note that this is only for the disk, the dust torus emission is reprocessed so it should be basically isotropic at high distance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, I have included the cosine in the SED calculation (in order to obtain the correct nuFnu depending on the viewing angle) and added the factor 2 in the luminosity calculations
d_L = Distance(z=z).to("cm") | ||
F_nu = sed / nu | ||
L = 4 * np.pi * np.power(d_L, 2) * np.trapz(F_nu, nu, axis=0) | ||
assert u.isclose(L, L_disk_test, atol=0 * u.Unit("erg s-1"), rtol=1e-2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you will also need to multiply L_disk_test times 2*cos (observation_angle)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test is done for observation angle=0, do you think it is work to include in the parametrizaiton also a second case with observation at an angle (to check those cos factors), or would it be overdoing the testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for asking to add it, made me realise that the new variable mu_s
was not propagated to all the functions computing the SEDs. Now it should be ok. Added three values of mu_s
to the tests.
…its luminosities in renormalisation and tests
Fix the black body SEDs of the Disk and the Dust Torus (Sourcery refactored)
Hello @jsitarek, so sorry for the huge delay in this, June has been full of meetings! I took into account in your suggestions, now I multiply the nuFnu SED by the cosine of the viewing angle (newly introduced I am not totally convinced it is sufficient to multiply by cosine to get the correct integral, maybe it works only when you simplify Beside the mathematical discussion, are you satisfied with the changes now? |
thx @cosimoNigro the function is used to determine the SED of the whole disk at the observer, i.e. you integrate the full disk, and then I think cos should work fine (it is also there in the formula in the reference that you provided. As far as I can understand it , the cos is there due to the black body property, it simply emits more perpendicularly to the surface than at an angle to it. And since the solid angle in which you see the disk is very small, also the change of distances along it does not matter. The situation gets more complicated when you are close to the disk, and see the emission from its different points at different angles, than you also need to take into account that with a given small solid angle you will also catch a different part of the disk surface depending on this viewing angle. This needs to be taken into account when computing EC and absorption from disk, but for just showing the disk SED at the observer is not important |
if you compare Eq. 10 of the paper and the formula in the docstrings I wrote it's clear they are simplifying the factor ( Let me know if you are satisfied with the changes (you can merge if you are). Sorry again for taking so long. |
Hi @cosimoNigro cos angle to the surface normal is always there, but only if the observer is far away from the disk it is the same angle as the observation angle. If you need to calculate it close to the disk, not only the distances will vary, but also each part of the disk will be observed at a different angle. I think this function is only meant for plotting SED for distant observer, then I suggest that the doc string is revised to take this into account, and give the cosine explicitely. If you need a similar function for computing the density of the photons from the disk either for IC or for pair production, this is more complicated, but since those functions were implemented from Dermen's and Finke's paper they should already have this cosine hidden somewhere. |
What about now? About this:
looking at the formulas that we re-written and the schemes we draw I think it is always assumed that the disk axis matches the jet axis, which I think it's fine for most calculations. It's the observer that is in turn inclined to an angle |
Sorry for being picky, but I think that the part 1+ R^2 /dL^2 is really confusing here. The formula is correct only if R<< dL, so I would suggest to skip this (1+...). |
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.24%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
I think it is fine now, thx |
I think you have to accept my changes or accept the review, it still says "changes requested". |
right, sorry, I just accepted the review and merged |
Dear @jsitarek,
this PR implements a correct multi-temperature black body SED for the Disk and a (single-temperature) black body SED for the DT.
I renormalise these SEDs to the respective luminosities of the Disk and DT, i.e. I impose that the integral luminosity of the SED has to be equal to the luminosity of the disk (or the luminosity of the DT).
I added these checks of the luminosity to the tests.
This should solve #23.
Changes in files other than
targets.py
andtest_targets.py
are unrelated, and need no checking, they are mostly corrections of the docstrings.Here a snippet to test the changes, developed on the notebook you posted in issue #23 (
checkout fix_disk_and_dt_bb
)