search

zfit / phasespace

Phase space generation implemented in TensorFlow
https://phasespace.rtfd.io
BSD 3-Clause "New" or "Revised" License
24 stars 9 forks source link

Implement repr method. #25

Closed apuignav closed 5 years ago

apuignav commented 5 years ago

Address an issue raised in #22.

@eduardo-rodrigues @mayou36 What do you think about this repr?

eduardo-rodrigues commented 5 years ago

It looks good to me, @apuignav (modulo our discussion on whether GenParticle could be a better name).

jonas-eschle commented 5 years ago

About the mass, we could include it if fixed, and otherwise something like "not fixed" or similar. It may helps debugging if a mass was set wrong. Otherwise looks good to me

apuignav commented 5 years ago

Not a bad idea. Let me try.

apuignav commented 5 years ago

Mass info has been added, but I had to add a _mass_value member to Particle in order to avoid the transformation to a tensor. However, if a tensor is passed this will fail

@mayou36 how would you deal with this?

jonas-eschle commented 5 years ago

To me, the mass in __repr__ is a nice to have information. If it is a number, fine. If not, not. If a Tensor, it's a Tensor (and it can be fixed or variable in principle).

So I would not guess. We could in __str__ add a sess call to the Tensor, but it's more trouble I guess (side-effects? expensive calls?).

Looks good to me, is what I would have thought.

And I would change the name once we change to GenParticle

apuignav commented 5 years ago

OK, then let's marge and do the move to GenParticle in a separate PR.