madgraph5 / madgraph4gpu

GPU development for the Madgraph5_aMC@NLO event generator software package
30 stars 33 forks source link

Fix bug in GPUFOHelasCallWriter format_coupling #822

Closed valassi closed 6 months ago

valassi commented 7 months ago

Hi @oliviermattelaer this is the PR closing #821 (This is also part of the fix for #820 and I will include it in PR #825 I think) Can you please review? Thanks Andrea

oliviermattelaer commented 6 months ago

Yes this is perfect. I guess replacing the "==" by "is" would also have done the trick. But while the above would have avoid to define one variable, this one has the advantages of clarity.

The only thing that one might consider if is the line

if alias == self.couporderindep: # bug #821! this is incorrectly true when all all dictionaries are empty!

are needed or not, (they certainly do not hurt). But I would advocate to at least modify them to

if alias == self.couporderindep: # bug #821! this is incorrectly true when both dictionaries are empty!

valassi commented 6 months ago

Yes this is perfect. I guess replacing the "==" by "is" would also have done the trick. But while the above would have avoid to define one variable, this one has the advantages of clarity.

Thanks Olivier! Interesting, I had not thought of "is"... But yes, let;s keep the more verbose and more clear version.

The only thing that one might consider if is the line ###if alias == self.couporderindep: # bug #821! this is incorrectly true when all all dictionaries are empty! are needed or not, (they certainly do not hurt). But I would advocate to at least modify them to ###if alias == self.couporderindep: # bug #821! this is incorrectly true when both dictionaries are empty!

Yes I have done this.

valassi commented 6 months ago

see above.

I let you decide, in any case this is good to go!

Ok I will merge this when the CI is done, just to get one out of the way. I will then update the other PRs.