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

Add an observable relevant for FPF studies #161

Merged
merged 3 commits into from
Dec 5, 2022
Merged

Conversation

Radonirinaunimi
Copy link
Member

For the FPF studies we need a definition of the double differential cross section that is not available yet. This PR just adds such a definition.

@@ -5,6 +5,7 @@
from .esf import ESFInfo
from .result import ESFResult, EXSResult

INV_GEV_TO_PB = 3.8937966e8 # pb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanjona, the conversion between pb and cm2 is almost trivial, and in any case I would store in a separate constant.

As you can see, we have now two constants, with a mantissa that is the same up to $10^{-6}$, and the difference is just the exponent (so you multiply this by 1e2 and you obtain essentially next line...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does make sense! I have removed the explicit definition of the constant but rather kept the variable name in the code just to make it explicit which conversion is being done XD.

@alecandido
Copy link
Member

alecandido commented Dec 2, 2022

Benchmarks are failing because they should not run, I lost track where they have been introduced (there is a long discussion going on since 2 years with @felixhekhorn, I hope I will fix this, sooner or later...).

Apart the comment above, the rest looks good to me.

When you are ready (i.e. replied to the comment), just ask again for review and I will approve :)

@alecandido alecandido mentioned this pull request Dec 2, 2022
3 tasks
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2022

Codecov Report

Merging #161 (1e8e2b5) into master (bbf33af) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
+ Coverage   70.36%   70.39%   +0.02%     
==========================================
  Files          90       90              
  Lines        4262     4266       +4     
==========================================
+ Hits         2999     3003       +4     
  Misses       1263     1263              
Flag Coverage Δ
unittests 70.39% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
src/yadism/esf/exs.py 77.77% <100.00%> (+1.50%) ⬆️

@Radonirinaunimi
Copy link
Member Author

Benchmarks are failing because they should not run, I lost track where they have been introduced (there is a long discussion going on since 2 years with @felixhekhorn, I hope I will fix this, sooner or later...).

Apart the comment above, the rest looks good to me.

When you are ready (i.e. replied to the comment), just ask again for review and I will approve :)

I was indeed wondering about the failing benchmarks but knew that this surely had nothing to do with the modifications.

@Radonirinaunimi
Copy link
Member Author

Thanks! I will merge this then now.

@Radonirinaunimi Radonirinaunimi merged commit 05c5acb into master Dec 5, 2022
@Radonirinaunimi Radonirinaunimi deleted the FPF-XSEC branch December 5, 2022 07:52
@felixhekhorn felixhekhorn added enhancement New feature or request physics physics features labels Dec 5, 2022
@@ -67,6 +67,10 @@ def xs_coeffs(kind, y, x=None, Q2=None, params=None):
ym = 0
yp = 1.0
yL = y**2 / (2 * (y**2 / 2 + (1 - y) - (mn * x * y) ** 2 / Q2))
elif kind == "XSFPFCC":
INV_GEV_TO_PB = GEV_CM2_CONV / 100.0 # Pb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the comment should be rather pb (note the small p - as in pico) (just to not confuse with lead 🙃 )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To really nitpick Pb in this context would mean "PetaBarn", instead of "picoBarn", according to SI.

Copy link
Member Author

@Radonirinaunimi Radonirinaunimi Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely a fair point! In addition, in what we are doing we also need the observable name added to the observable_name.py. Let me restore this branch and add these changes here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now in #163.

@Radonirinaunimi Radonirinaunimi restored the FPF-XSEC branch December 6, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request physics physics features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants