fix!: update PDG values in particle_list #202

redeboer · 2020-08-12T15:57:19Z

No description provided.

WARNING: This affects the number of solutions of D0 -> K0S K+ K- tests/channels/test_d0_to_kskpkm.py

codecov · 2020-08-13T06:59:10Z

Codecov Report

Merging #202 into master will increase coverage by 0.34%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #202      +/-   ##
==========================================
+ Coverage   88.28%   88.62%   +0.34%     
==========================================
  Files          22       22              
  Lines        2851     2858       +7     
  Branches      663      665       +2     
==========================================
+ Hits         2517     2533      +16     
+ Misses        190      184       -6     
+ Partials      144      141       -3

Flag	Coverage Δ
#unittests	`88.62% <100.00%> (+0.34%)`	⬆️

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

Impacted Files	Coverage Δ
expertsystem/data.py	`98.34% <100.00%> (+5.81%)`	⬆️
expertsystem/io/xml/_build.py	`85.96% <0.00%> (-3.51%)`	⬇️
expertsystem/state/particle.py	`88.61% <0.00%> (+0.22%)`	⬆️

redeboer · 2020-08-13T06:59:50Z

@spflueger I'm only worried about 6226f16 because of the required changes in tests/channels/test_d0_to_kskpkm.py

spflueger · 2020-08-13T07:11:11Z

@spflueger I'm only worried about 6226f16 because of the required changes in tests/channels/test_d0_to_kskpkm.py

Hmm I can take a look.

spflueger

A faulty definition of the charged a0(980) particles make the D decay test fail. Currently a warning message is print out in such a case (quantum number solutions are found, but no matching particle can be inserted, due to a wrongly defined particle). I opened issue #205 to avoid having faulty particles in general. It would be a good options to include such a test in this PR, to ensure all particle definitions are correct.

expertsystem/particle_list.yml

spflueger · 2020-08-13T12:45:42Z

So implementing the test as requested by #205 is to tedious currently. Once we switch the rules to use QuantumState like objects it will be much easier. So I would postpone #205 until later

redeboer · 2020-08-13T12:47:35Z

So implementing the test as requested by #205 is to tedious currently.

Why is it tedious? I propose to:

add a hypercharge property to QuantumState (works for both QuantumState[float] and QuantumState[Spin]
do the Gell-Mann–Nishijima formula check in the constructor of Particle.

wgradl · 2020-08-13T12:57:05Z

A faulty definition of the charged a0(980) particles make the D decay test fail. Currently a warning message is print out in such a case (quantum number solutions are found, but no matching particle can be inserted, due to a wrongly defined particle). I opened issue #205 to avoid having faulty particles in general. It would be a good options to include such a test in this PR, to ensure all particle definitions are correct. > QuantumNumbers: Spin: 0 Charge: -1 Parity: +1 - IsoSpin: { Value: 1, Projection: -1 } The projection -1 was correct > QuantumNumbers: Spin: 0 Charge: 1 Parity: +1 - IsoSpin: { Value: 1, Projection: 1 } The projection +1 was correct. This is the reason why the `D0 -> K0 K+ K-` only finds 3 solutions now.

hmmm ... consider the decay D0 --> a0(980)+ K- on the quark level: [c ubar] --> [s ubar] W+ --> [s ubar] [u dbar] ~ K- a0+ the a0+ then decays like this: [u dbar] --> [u (sbar s) dbar] --> [u sbar] [s dbar] ~ K+ K0_bar so, the correct decay to consider would be D0 --> K0_bar K+ K-. No?

…

-- Wolfgang

-- Prof Dr Wolfgang Gradl University of Mainz BES-III, BABAR & A2@MAMI Institut für Kernphysik Tel. +49-6131-39-25871

spflueger · 2020-08-13T13:00:02Z

So implementing the test as requested by #205 is to tedious currently.

Why is it tedious? I propose to:

You have to convert the Particle to a dict of Enums etc

add a hypercharge property to QuantumState (works for both QuantumState[float] and QuantumState[Spin]

Since hypercharge can be calculated from the other QN already, this info is redundant and just adds one more point of inconsistency. Hence I oppose that

do the Gell-Mann–Nishijima formula check in the constructor of Particle.

That is a good point.

spflueger · 2020-08-13T13:08:11Z

hmmm ... consider the decay D0 --> a0(980)+ K- on the quark level: [c ubar] --> [s ubar] W+ --> [s ubar] [u dbar] ~ K- a0+ the a0+ then decays like this: [u dbar] --> [u (sbar s) dbar] --> [u sbar] [s dbar] ~ K+ K0_bar so, the correct decay to consider would be D0 --> K0_bar K+ K-. No?

@wgradl Maybe there is slight misunderstanding. We are currently testing D0 --> K0_bar K+ K-. Just the isospin of a0(980)+/- were defined incorrectly in this PR, which made the test fail. So everything is ok.

wgradl · 2020-08-13T13:13:31Z

> hmmm ... consider the decay D0 --> a0(980)+ K- on the quark level: [c ubar] --> [s ubar] W+ --> [s ubar] [u dbar] ~ K- a0+ the a0+ then decays like this: [u dbar] --> [u (sbar s) dbar] --> [u sbar] [s dbar] ~ K+ K0_bar so, the correct decay to consider would be D0 --> K0_bar K+ K-. No? @wgradl Maybe there is slight misunderstanding. We are currently testing `D0 --> K0_bar K+ K-`. Just the isospin of a0(980)+/- were defined incorrectly in this PR, which made the test fail. So everything is ok.

@spflueger: ok, I see. Your inital comment mentioned D0 --> K0 K+ K-, so I was a bit puzzled. Nevertheless, the isospin projection of the a0+ should be +1, just as for the pi+ (because it has identical quark contents). If that makes the tests fail, then something else has gone awry.

spflueger · 2020-08-13T14:32:06Z

hmmm ... consider the decay D0 --> a0(980)+ K- on the quark level: [c ubar] --> [s ubar] W+ --> [s ubar] [u dbar] ~ K- a0+ the a0+ then decays like this: [u dbar] --> [u (sbar s) dbar] --> [u sbar] [s dbar] ~ K+ K0_bar so, the correct decay to consider would be D0 --> K0_bar K+ K-. No? @wgradl Maybe there is slight misunderstanding. We are currently testing D0 --> K0_bar K+ K-. Just the isospin of a0(980)+/- were defined incorrectly in this PR, which made the test fail. So everything is ok.
@spflueger: ok, I see. Your inital comment mentioned D0 --> K0 K+ K-, so I was a bit puzzled. Nevertheless, the isospin projection of the a0+ should be +1, just as for the pi+ (because it has identical quark contents). If that makes the tests fail, then something else has gone awry.

Ah I see, very valuable comment. Currently the test just checks if this is generally possible. Since it will also try pure weak interactions (for both vertices/nodes) it will also work with a K0. Hence I used the terms K0 and K0_bar abit sloppy here.

I conclude two important points from your comment:

we should make the second decay vertex strong interaction only. This would actually forbid a K0.
we can think about implementing CKM matrix (roughly) to further rate different decay channels based on their probability or strength

Needed because code coverage drops in the following commits

spflueger · 2020-08-13T15:39:11Z

expertsystem/data.py

@@ -215,6 +216,37 @@ def __repr__(self) -> str:
        return f"{self.__class__.__name__}{self.name, self.pid, self.state, self.mass, self.width}"


+def compute_gellmann_nishijima(state: QuantumState) -> Optional[float]:


Better change this to return a int

I did that at first, but then figured the cast to int that would be required here is implicit behaviour. Afaik formula itself holds just as well for float charges.

But the charge from the QuantumState or your tests are using int. So then a potential cast to int will be done at that point of comparison. Hence I would rather put it in that function. It might hold for float charges, but actually measurable particle has float charge. So you could also filter this problem there. if (charge).is_integer(): return int(charge)

I think the filtering should take place here:

expertsystem/expertsystem/data.py

Lines 192 to 195 in c0abac0

if (

state.isospin is not None

and compute_gellmann_nishijima(state) != state.charge

):

This check will fail if the GM fomula returned a float that is not an integer.

if (charge).is_integer(): return int(charge)

To be clear: this would also mean raising an exception if the charge is not castable to int. For me that is unexpected behaviour in a function that is just supposed to compute the Gell-Mann–Nishijima formula (as its name states).

Hmm, you have a point. So I think you are right, we do not have to put any more assumptions in there. I just want to make sure we are actually catching out the ill defined particles.

I think because particle.charge is of type int, we are on the safe side.

Particle instances with non-castable float charge raise an exception, even if they comply with the Gell-Mann–Nishijima formula. QuantumState instances with weird charges are allowed, but there's no check on GM formula there anyway.

And mypy would fish out float charges anyways right?

Yes, but mypy isn't checked on outside scripts, it's only used as a consistency check for the framework and tests themselves. But for a user running, say, a Jupyter notebook, an exception would be raised for any Particle that has unphysical charge (as per Gell-Mann–Nishijima formula + cast check). QuantumState remains an internal matter, however.

tests/data/test_particle.py

fix: remove XX particle

7ead02d

redeboer added Bug Something isn't working 🔨 Maintenance Refactoring that doesn't affect the interface labels Aug 12, 2020

redeboer self-assigned this Aug 12, 2020

redeboer requested a review from spflueger August 12, 2020 15:58

redeboer added 7 commits August 13, 2020 08:46

fix: rename to Sigma and Lambda

63a0880

fix: PDG IDs

bdcbc77

fix: particle widths

fde89b5

fix: particle masses

5ad04fc

fix: isospin values

6226f16

WARNING: This affects the number of solutions of D0 -> K0S K+ K- tests/channels/test_d0_to_kskpkm.py

fix: G-parity values

e2be746

fix: C-parity values

1421c56

style: fix spelling Python file names

241b898

redeboer changed the title ~~fix!: update PDG values in particle_list.*xml~~ fix!: update PDG values in particle_list.*ml Aug 13, 2020

redeboer changed the title ~~fix!: update PDG values in particle_list.*ml~~ fix!: update PDG values in particle_list Aug 13, 2020

ci: fix pre-commit hook name test_* check

9d0913b

Leongrim mentioned this pull request Aug 13, 2020

feat: create ParticleCollection from PDG #190

Merged

6 tasks

spflueger suggested changes Aug 13, 2020

View reviewed changes

expertsystem/particle_list.yml Outdated Show resolved Hide resolved

expertsystem/particle_list.yml Show resolved Hide resolved

redeboer added 2 commits August 13, 2020 17:07

ci: run mypy separately in tox style job

3017f8d

fix: isospin projection of a0(980)+-

6e5ff35

redeboer added 3 commits August 13, 2020 17:07

ci: equalize tox test coverage to codecov

6246bad

Needed because code coverage drops in the following commits

feat: add Gellmann-Nishijima formula

446611a

feat: implement Gell-Mann–Nishijima exception

c0abac0

redeboer mentioned this pull request Aug 13, 2020

Check consistency of particle definitions #205

Closed

spflueger reviewed Aug 13, 2020

View reviewed changes

redeboer added 3 commits August 13, 2020 18:11

ci: increase patch test coverage

d0206f1

test: add baryon test case

83167cd

fix: remove __eq__ test ParticleCollection

458c5ea

spflueger approved these changes Aug 13, 2020

View reviewed changes

redeboer merged commit 3c3a20a into ComPWA:master Aug 13, 2020

redeboer deleted the fix-particle_list-content branch August 13, 2020 16:58

This was referenced Apr 6, 2021

Implement CKM matrix functionality ComPWA/qrules#15

Open

Add functionality to assign interactions to individual interaction nodes ComPWA/qrules#16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: update PDG values in particle_list #202

fix!: update PDG values in particle_list #202

redeboer commented Aug 12, 2020

codecov bot commented Aug 13, 2020 •

edited

Loading

redeboer commented Aug 13, 2020 •

edited

Loading

spflueger commented Aug 13, 2020

spflueger left a comment

spflueger commented Aug 13, 2020

redeboer commented Aug 13, 2020

wgradl commented Aug 13, 2020 via email

spflueger commented Aug 13, 2020 •

edited

Loading

spflueger commented Aug 13, 2020

wgradl commented Aug 13, 2020 via email

spflueger commented Aug 13, 2020

spflueger Aug 13, 2020

redeboer Aug 13, 2020 •

edited

Loading

spflueger Aug 13, 2020

redeboer Aug 13, 2020

redeboer Aug 13, 2020

spflueger Aug 13, 2020

spflueger Aug 13, 2020

redeboer Aug 13, 2020 •

edited

Loading

spflueger Aug 13, 2020

redeboer Aug 13, 2020

		@@ -215,6 +216,37 @@ def __repr__(self) -> str:
		return f"{self.__class__.__name__}{self.name, self.pid, self.state, self.mass, self.width}"


		def compute_gellmann_nishijima(state: QuantumState) -> Optional[float]:

	if (
	state.isospin is not None
	and compute_gellmann_nishijima(state) != state.charge
	):

fix!: update PDG values in particle_list #202

fix!: update PDG values in particle_list #202

Conversation

redeboer commented Aug 12, 2020

codecov bot commented Aug 13, 2020 • edited Loading

Codecov Report

redeboer commented Aug 13, 2020 • edited Loading

spflueger commented Aug 13, 2020

spflueger left a comment

Choose a reason for hiding this comment

spflueger commented Aug 13, 2020

redeboer commented Aug 13, 2020

wgradl commented Aug 13, 2020 via email

spflueger commented Aug 13, 2020 • edited Loading

spflueger commented Aug 13, 2020

wgradl commented Aug 13, 2020 via email

spflueger commented Aug 13, 2020

Choose a reason for hiding this comment

redeboer Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redeboer Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 13, 2020 •

edited

Loading

redeboer commented Aug 13, 2020 •

edited

Loading

spflueger commented Aug 13, 2020 •

edited

Loading

redeboer Aug 13, 2020 •

edited

Loading

redeboer Aug 13, 2020 •

edited

Loading