Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve consistency of calculation functions over sets of QuantityType values #4562

Open
andrewfg opened this issue Jan 18, 2025 · 0 comments · May be fixed by #4563
Open

Improve consistency of calculation functions over sets of QuantityType values #4562

andrewfg opened this issue Jan 18, 2025 · 0 comments · May be fixed by #4563

Comments

@andrewfg
Copy link
Contributor

andrewfg commented Jan 18, 2025

Indeed. And as mentioned above, it applies to ALL calculations based on sets of items with mixed units. So to be clear this is not really an issue with toInvertibleUnit .. it is rather an issue with the 'sum' algorithm when the set includes items having a non zero point offset.

That is not correct, as the calculation functions will be done on the absolute scale since #3792. So there is no unpredictability.

Looking at the code of the arithmetic group functions, it looks like items of another dimension than the first are excluded from the sum. So depending on the unit of the first item, you will have a different set of items used in the function if there are invertible units in it. It would probably be better to use the unit of the group base item, instead of the unit of the first item, and then allow the reciprocal unit in the calculation after conversion to the base unit. I notice in the history the use of toInvertibleUnit in that method has explicitly been added to cope with color temperature, but in my view it does not work at all at this time (and that is from reading code, not testing). Also it is not used in all methods, only a few.

Originally posted by @mherwege in #4561 (comment)

@andrewfg andrewfg linked a pull request Jan 18, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant