mctools / mcpl

Monte Carlo Particle Lists
https://mctools.github.io/mcpl/
Other
29 stars 13 forks source link

pymcpltool --stat occasionally gives incorrect sum(weights) with python3 #46

Closed tkittel closed 5 years ago

tkittel commented 5 years ago

This has been seen for years, but becoming more important now due to the final python2->python3 migration taking place in many projects.

tkittel commented 5 years ago

Got it finally!!! The bug was:

for s,sc in collected_stats.items():
    if weight_sum is None:
        weight_sum = sc['integral']

which should be:

for s,sc in collected_stats.items():
    if weight_sum is None and s!='weight':
        weight_sum = sc['integral']

The weight stat is special, as it represents the weights themselves, with unity "weights" (i.e. unweighted). The integral of any other stat would give the correct sumw result. In python2 we were so "lucky" that the first encountered stat in the iteration was not the weight stat, but in python3 hashing is randomised by default, so once in a while the first encountered stat would indeed be the weight stat => effectively getting count(entries) rather than sumw.

The fix will be included in the next ncrystal release (1.3.0) which should be released within ~1 week. After that, we should remember to update the PyPI mcpl package as well.

tkittel commented 5 years ago

MCPL 1.3.0 was released fixing this!