How should UAtom uncertainty components with zero weight be handled?
#321
Unanswered
jagerber48
asked this question in
Q&A
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
I am working on the branch that has the
LinearCombo/Variable/AffineScalarFunctoUAtom/UCombo/UFloatrefactor and I'm coming across an edge case with how theUComboworks. The issue has to do with when the weight of a certainUAtomwithin aUCombois equal to zero. This discussion is related to #283.The code in question is around here https://github.com/lmfit/uncertainties/pull/262/files#diff-47117210822144d1b82b5f22ecfe2d43d83ac2aa86907bd24fb205a698881960R52
In this new architecture
UAtomis essentially just a unique ID (UUID) generated using uuid4. Unless twoUAtomhave the same UUID, they are unequal. UnequalUAtomobjects represent independent, unit variance, zero mean random variables.UCombois thought of a linear combination ofUAtomobjects. Slightly more concretely, aUCombois a mapping fromUAtomobjects to float weights.UComboobjects can be added together and multiplied by scalars. These operations support the linear error propagation arithmetic which is the heart of theuncertaintiespackage. TheUComboobject supports the lazy expansion of its mapping. This is what allows many arithmetic operations to be performed onUFloatobject with linear, rather than quadratic, computational complexity. If you want to calculate thestd_devor__hash__of aUComboobject theUComboobject must be "expanded" into a simpledictmappingUAtomtofloatweights.__eq__is calculated based on__hash__.UFloathas a floatnominal_valueattribute and aUCombouncertaintyattribute.Before expansion, the
UCombo's main attribute is theucombo_tuple. This has the typeThis is like a collection of mappings from either
UAtomobjects or other nestedUComboobjects tofloatweights. When theexpandedproperty is accessed via thestd_devproperty or__hash__method theucombo_tupleis recursively walked through and all like-terms (sameUAtom) are collected and anexpanded_dictthat mapsUAtomtofloatweights in returned.Right now, in the spirit of #283 and not special casing 0, the behavior is that a
UAtomcan have a weight of exactly zero. In this case theUAtomdoesn't contribute to thestd_dev. However, the edge case issue that I am having, is that twoUFloatcan have the samenominal_valueand the sameexpanded_dictUP TO inclusion ofUAtomwith 0 weights. Thehashis calculated by hashing each key/value pair in theexpanded_dict, so even if twoUCombodiffer only by inclusion ofUAtomwith weight 0, thehashwill come out different and they will compare not equal. This is NOT in the spirit of having equality model random variables that we've discussed elsewhere. Indeed this test fails:The resolutions that I see are
UAtomwith zero weight from the__hash__calculation. This will fix the test, but it has the problem that when users view theexpanded_dictit may look different between two different but equalUComboobjects.UAtomwith zero weight from theexpanded_dict. At the end of the expansion calculation I can check forUAtomwith weight 0 and exclude them from theexpanded_dict. This means theexpanded_dictforzeroandx_zeroUFloatobjects above would be empty. The disadvantages here are that we are special casing zero again which changes how some of the code is structured and that there might be a performance implication for this check during theexpandedproperty calculation.Specifically, when I implement option 2, many tests start failing. In these cases I believe the source code is behaving fine and that the tests are simply failing because they are written WITHOUT special casing this zero weight case. The tests can be reasonably rewritten to special case the zero weight case, but it's outside of the spirit of the path I'm taking out of #283 to try to avoid special casing zero.
Because the downside of option 1 seems pretty problematic, I am leaning towards exploring option 2 which involves rewriting tests and doing performance analysis. I'm curious for others to understand and comment on the issue and give opinions on my proposed resolutions or propose other resolutions.
All reactions