cosimoNigro / agnpy

Modelling jetted Active Galactic Nuclei radiative processes with python
https://agnpy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
48 stars 33 forks source link

Absorption mu s dependent #81

Closed jsitarek closed 3 years ago

jsitarek commented 3 years ago

absorption in DT for the case of gamma rays not moving along the jet included also a few tests of the newly added methods

codecov[bot] commented 3 years ago

Codecov Report

Merging #81 (15d39d9) into master (489f1cc) will decrease coverage by 0.67%. The diff coverage is 99.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage   93.97%   93.30%   -0.68%     
==========================================
  Files          30       31       +1     
  Lines        1727     1896     +169     
==========================================
+ Hits         1623     1769     +146     
- Misses        104      127      +23     
Flag Coverage Δ
unittests 93.30% <99.44%> (-0.68%) :arrow_down:

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

Impacted Files Coverage Δ
agnpy/absorption/absorption.py 94.70% <97.82%> (-3.80%) :arrow_down:
agnpy/tests/test_absorption.py 90.41% <100.00%> (-9.59%) :arrow_down:
agnpy/tests/test_geometry.py 100.00% <100.00%> (ø)
agnpy/utils/geometry.py 100.00% <100.00%> (ø)

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 489f1cc...15d39d9. Read the comment docs.

cosimoNigro commented 3 years ago

Thanks a lot, @jsitarek, will check tomorrow! Can I ignore PR #79? You modified the same lines in this PR, in the computation of mu_star_shell. I am afraid if I merge #79 it will create a conflict (i.e. two PRs modifying the same lines).
Apologies for not checking #79 before.

jsitarek commented 3 years ago

Hi @cosimoNigro

sorry, probably I forked a new branch from already modified master (I should have made a dedicated branch for this fix in PR #79) and hence ended up with the same changes also in this branch. Please check this one first and if github does not realize that the changes are the same then we cancel the PR #79.

cosimoNigro commented 3 years ago

Dear @jsitarek,

thanks a lot for already taking care of this. I have left a small review, very minor comments only.

What I would like to ask you is to provide some written formulas with a sketch for the calculation you are introducing. I could only verify x_re_ring_mu_s in the case in which the target and emitted photon are in the same plane photo5308020145937035691 Can you please provide something like this for the formulas you introduced in utils.geometry? I do not want you to write them in latex, you can just send me a decent photo / scan of your calculation and I will take care of introducing them in the paper draft / docs. Thank you!

jsitarek commented 3 years ago

sure, I will provide the formulae (actually probably better that I write it in latex, my handwriting is terrible :-) )

sourcery-ai[bot] commented 3 years ago

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 3.80%.

Quality metrics Before After Change
Complexity 0.24 ⭐ 0.66 ⭐ 0.42 👎
Method Length 66.19 🙂 77.54 🙂 11.35 👎
Working memory 11.99 😞 12.67 😞 0.68 👎
Quality 66.45% 🙂 62.65% 🙂 -3.80% 👎
Other metrics Before After Change
Lines 647 958 311
Changed files Quality Before Quality After Quality Change
agnpy/absorption/absorption.py 69.15% 🙂 66.52% 🙂 -2.63% 👎
agnpy/tests/test_absorption.py 60.96% 🙂 55.93% 🙂 -5.03% 👎
agnpy/utils/geometry.py 81.76% ⭐ 76.70% ⭐ -5.06% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
agnpy/tests/test_absorption.py TestAbsorptionMuS.test_tau_dt_mu_s_far 0 ⭐ 285 ⛔ 18 ⛔ 42.88% 😞 Try splitting into smaller methods. Extract out complex expressions
agnpy/absorption/absorption.py Absorption.evaluate_tau_ss_disk 0 ⭐ 233 ⛔ 18 ⛔ 45.35% 😞 Try splitting into smaller methods. Extract out complex expressions
agnpy/tests/test_absorption.py TestAbsorptionMuS.test_tau_dt_mu_s_simple 0 ⭐ 210 ⛔ 15 😞 49.80% 😞 Try splitting into smaller methods. Extract out complex expressions
agnpy/absorption/absorption.py Absorption.evaluate_tau_blr 0 ⭐ 147 😞 15 😞 55.37% 🙂 Try splitting into smaller methods. Extract out complex expressions
agnpy/absorption/absorption.py Absorption.evaluate_tau_dt_mu_s 0 ⭐ 140 😞 15 😞 56.14% 🙂 Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The 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!

Let us know what you think of it by mentioning @sourcery-ai in a comment.

cosimoNigro commented 3 years ago

Thanks, looks good for now, I am merging. If we want to change something we can open another PR. Thanks!