Skip to content

Change max_as during FONLL-B DIS fk calculation#27

Merged
andreab1997 merged 13 commits into
mainfrom
fix_fonllb
Jul 12, 2022
Merged

Change max_as during FONLL-B DIS fk calculation#27
andreab1997 merged 13 commits into
mainfrom
fix_fonllb

Conversation

@andreab1997
Copy link
Copy Markdown
Contributor

We need pineko to change the max_as value from 1 to 2 when computing FONLL-B DIS fktables.
Solves #26

@felixhekhorn felixhekhorn changed the base branch from main to feature/sv July 5, 2022 12:23
Comment thread src/pineko/check.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 11, 2022

Codecov Report

Merging #27 (a421944) into main (9fea7f4) will increase coverage by 0.75%.
The diff coverage is 61.53%.

❗ Current head a421944 differs from pull request most recent head 6fe4865. Consider uploading reports for the commit 6fe4865 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
+ Coverage   40.00%   40.75%   +0.75%     
==========================================
  Files          15       15              
  Lines         520      530      +10     
==========================================
+ Hits          208      216       +8     
- Misses        312      314       +2     
Flag Coverage Δ
unittests 40.75% <61.53%> (+0.75%) ⬆️

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

Impacted Files Coverage Δ
src/pineko/theory.py 19.87% <0.00%> (-0.25%) ⬇️
src/pineko/check.py 66.66% <100.00%> (+9.52%) ⬆️

Comment thread src/pineko/check.py Outdated
@delete-merged-branch delete-merged-branch Bot deleted the branch main July 11, 2022 13:36
@andreab1997
Copy link
Copy Markdown
Contributor Author

same here: should I add tests here?

@alecandido
Copy link
Copy Markdown
Collaborator

same here: should I add tests here?

I'm not connecting to what "same" is referred, but definitely yes. Add as many relevant tests as you can!

@andreab1997
Copy link
Copy Markdown
Contributor Author

same here: should I add tests here?

I'm not connecting to what "same" is referred, but definitely yes. Add as many relevant tests as you can!

yes sorry, I forgot to tag the other PR. Same of #24

@felixhekhorn felixhekhorn changed the base branch from feature/sv to main July 12, 2022 12:24
@felixhekhorn felixhekhorn added the bug Something isn't working label Jul 12, 2022
@andreab1997
Copy link
Copy Markdown
Contributor Author

again, I am asking the review only for the test

Comment thread src/pineko/check.py Outdated
Comment thread src/pineko/theory.py Outdated
Comment thread src/pineko/theory.py Outdated
Comment thread src/pineko/check.py Outdated
Comment thread src/pineko/check.py Outdated
Comment thread src/pineko/check.py Outdated
look for a generic lepton

Co-authored-by: Alessandro Candido <candido.ale@gmail.com>
@alecandido
Copy link
Copy Markdown
Collaborator

alecandido commented Jul 12, 2022

For me now would be even fine, but tests are failing, I guess because I forgot a not in the if condition:

if not (10 < abs(el[1]) < 17):

@alecandido
Copy link
Copy Markdown
Collaborator

If tests will pass, you can merge @andreab1997

@andreab1997
Copy link
Copy Markdown
Contributor Author

For me now would be even fine, but tests are failing, I guess because I forgot a not in the if condition:

if not (10 < abs(el[1]) < 17):

Yes of course

@andreab1997 andreab1997 merged commit aa1a068 into main Jul 12, 2022
@delete-merged-branch delete-merged-branch Bot deleted the fix_fonllb branch July 12, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants