Particle.pdgid should return int

[HDembinski]

Context: I use Numba a lot, so it would be great if particle would work with Numba directly, but I guess that this is probably a more elaborate project. In the meantime as a workaround, I extract information from particle and store that in Python lists and dicts, which I then pass to the numba-compiled functions.

The attribute Particle.pdgid is of type particle.pdgid.pdgid.PDGID instead of int, which leads to unexpected issues. I argue it would be better to return a plain int. The type particle.pdgid.pdgid.PDGID emulates a plain int, so you won't notice the difference much in pure Python, but it is a different type nonetheless and Numba does not understand this type. If I prepare a list or dict of PDG IDs to pass to Numba, I need to call int(Particle.pdgid) explicitly, which is doable but irritating.

I argue that there is no benefit of Particle.pdgid being particle.pdgid.pdgid.PDGID here, because the extra information that can be queried from particle.pdgid.pdgid.PDGID is can already be queried from Particle.

[eduardo-rodrigues]

Thanks for bringing this up. I would be curious to see how you are using Particle with Numba, and what are issues part from this PDGID to ìnt` conversion, at least for my own education.

What you ask is not a totally trivial change. The immediate issue is that powerful searches such as

>>> # Find all neutral beauty hadrons
>>> Particle.findall(lambda p: p.pdgid.has_bottom and p.charge==0)

would no longer be possible with such a simple one-line selection function, and that's really bad IMO as that's the kind of area where the package shines.

Is that really such a trouble to do [int(p.pdgid) for p in Particle.findall(...)] at the top of a chunk of work?

@henryiii, what's your immediate reaction?

[henryiii]

My immediate reaction is a) it's easier to convert to int than it is to find and convert to PDGID, b) PDGID provides extra functionality, and duck types like an int (causing non-Numba users to be only inconvenienced by the change), and c) why not just teach Numba that a PDGID is an LLVM int? We could probably even add that to Particle pretty easily.

[henryiii]

Also, int(p) is a good way to get an integer particle number, IIRC, and is one less character than p.pdgid.

[eduardo-rodrigues]

Also, int(p) is a good way to get an integer particle number, IIRC, and is one less character than p.pdgid.

Yes, that's right.

[HDembinski]

Context: In my Numba combined function, I need to look up the mass of particles based on their PID and I need to look up how many strange quarks it has. My workaround is to generate some Numba Dicts outside the Numba function and fill those with the information, then I pass those Dicts to Numba functions. Sure, one can do this, but I noticed then and there, that particle is not designed to process large arrays of PIDs, like "take an array of PIDs and give me a new array with all the masses".

It is a weird design decision to make Particle not derive from PDGID, but allow int(Particle(...)), effectively treating Particle like a PDGID. If Particle is effectively a PDGID, it should offer the full superset of the interface of PDGID, in other words, it should derive from PDGID. If there is an obvious "is-a" relationship between Particle and PDGID (I think there is), then inheritance is the right tool. See e.g. Meyers Effective C++.

It is weird that .pdgid is not returning a plain int, because a PID is fundamentally a plain int. When I call a data attribute of a class that is to be used in numerical code, I expect it to return plain data types unless there is a good reason not to.

Perhaps you heard somewhere that composition is better than inheritance, but these statements are never generally true. You used composition here, but that choice is not natural. If Particle did derive from PDGID, it wouldn't have to offer access to PDGID via an attribute and we would have this discussion.

[HDembinski]

To stress this again, if Particle does not derive from PDGID, then it is very unnatural that int(Particle(...)) works, since Particle according to your design "is not" a PDGID.

[HDembinski]

More surprises which are a consequence of Particle not inheriting from PDGID: abs(Particle(...)) fails, but abs(PDGID(...)) works.

[henryiii]

Perhaps you heard somewhere that composition is better than inheritance

This was partially a historical result, rather than a conscious design. @eduardo-rodrigues mostly wrote PDGID, and I mostly wrote Particle. PDGID did not exist when Particle was designed, so it would be quite odd for Particle to inherit from it.

A 1.0 redesign could possibly include larger changes, and possibly include moving the particle search functionality out of Particle and into a Particles/ParticleTable class (have not thought about it at this point, maybe @eduardo-rodrigues has). Another option would be to reduce the correspondence between PDGID's and Particles, and instead treat pdgid as one of many possible representations, removing __int__().

PS: I really don't like APIs like "make a singleton-like class then use it" for a library supporting beginners, but we might be able to provide beginner friendly options and a "default" particle table.

The defense of the current design would be that the intent of int(Particle(...)) is clear - you are asking for a int representation of the current particle, and that by far most likely to be a PDG ID int (see option two, though, which might remove this). But a particle is not an int; it just has a explicit conversion to an int. abs(Particle(...)) fails (or should return a Particle, possibly?). Particle(...) + 1 has no meaning, since not all particles exist. PDGID() + 1 does, since it's just an int representing the ID of a particle. (PDGID(...) + 1).has_charm is a perfectly valid question. PDGID is valid for all possible ints, while Particle is a lookup-based object; the really do not have anything in common. Particle is not supposed to duck type as an int, while PDGID does (since it is an int). While PDGID is an int, but also has further information beyond an int.

Again, the real problem is that Numba is picky, and doesn't treat subclasses of ints as ints like normal Python does. But we could probably easily teach it otherwise (maybe even teaching it a few details about PDGIDs, in fact) if there's a strong enough desire for it.

PS: @eduardo-rodrigues , if you are interested in enabling all PDGID's methods on particle, but without turning Particles also into ints and causing Particle() + 1 to "work" but make an invalid particle, we can explore this; I have a simple idea for how this could be done.

[henryiii]

By they way, snippets or pointers to Numba code would be very interesting; it might help us decide how to support Numba more natively.

[henryiii]

PS: int(p) produces an int, and PDGID(p) produces a PDGID. p.pdgid is just a shortcut for PDGID(p).

Second reminder: you can also make Particles that do not have a valid PDGID by specifying all the properties. int(p) may fail.

[HDembinski]

Ok, I get why Particle does not derive from PDGID, but then I find it weird to allow int(Particle(...)). You want to teach people to use int(Particle(...).pdgid) then. Allowing int conversion for both PDGID and Particles smears this important distinction between the two.

[HDembinski]

Some pseudo-code of what would be useful to do in Numba (my actual code is more complex):

@nb.njit
def get_mass(array_of_pids):
    masses = np.empty(len(array_of_pids))
    for i, pid in enumerate(arrays_of_pids):
          m = Particle.from_pdgid(pid).mass
          masses[i] = m if m is not None else np.nan
    return masses

[eduardo-rodrigues]

Ok, I get why Particle does not derive from PDGID, but then I find it weird to allow int(Particle(...)). You want to teach people to use int(Particle(...).pdgid) then. Allowing int conversion for both PDGID and Particles smears this important distinction between the two.

Thanks for the reminder and apologies for forgetting to follow-up this thread. I will do so this week.

Yes, agreed that it's weird. I can't remember why that was introduced but I would very much like to remove Particle.__init__ asap. I'm sure I will give more reasoning when properly replying.

[eduardo-rodrigues]

As you say @HDembinski ,
"""
... particle is not designed to process large arrays of PIDs ...",like "take an array of PIDs and give me a new array with all the masses"
""".
But the important bit here is what you mean by large. If you are going to make this ID-mass match for tens of thousands or millions of entries in some ROOT file or alike, then the for-loop is going to be "slow". This being said, you would rather wrap things in a very simple cache function to avoid checking pion ID-mass matches and other very commonly found particles, for the sake of example.
Unless I'm misunderstanding your comment.

A lot has been said already about inheritance of Particle from PDGID. Yes, a particle is uniquely identified in particular in MC programs by its PDG ID. But a particle is not an int number that cleverly encodes various properties for the particle it relates to. A particle is quite a different beast - not talking here in terms of code.

I can see that inheritance would give us some functionality for free, such as the is_XXX properties of a PDG ID. Still, I very much prefer to keep the classes uninherited.

Agreed that Particle.__init__ can look rather awkward and I will just now create a PR to remove this. I don't think it will have an impact on users.

You mentioned inheritance and composition. I see your point. But the design was to have also a PDGID class separate from Particle, precisely because these can be different things. @henryiii has already dwell into that.

One could also put forward to actually have
Particle.pdgid -> int
Particle.to_pdgid -> PDGID
Would that be crazy or rather useful?

Now, this discussion is raising the fact that methods such as Particle.findall(...) return a list of Particle instances
whereas many other properties are simply for a Particle instance. In that respect the Particle class does two things.
Almost any user is happy with the functionality but we could create a ParticleTable (or alike) class in the future, as @henryiii suggests. This is a separate discussion, though.

Yes @henryiii, the idea of a particle table class came to my mind already, and precisely because of the behaviour discussed above. This could come in the future, sure, and I think we should give the idea a proper thought ...
BTW, I would actually like to release version 1.0.0 very soon, as the API has been stable to > 95% C.L. for a while.
What do you think?