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

Fix math functions for percent #329

Open
wants to merge 2 commits into
base: v3.x
Choose a base branch
from
Open

Conversation

StefanoD
Copy link

@StefanoD StefanoD commented Jan 19, 2024

Fixes #328

Note: I tried to alter all math functions, but this resulted in some unit tests failing. There might be some rework needed from somebody with more knowledge.

@nholthaus
Copy link
Owner

I think this may stem from a misunderstanding of how concentrations are intended to work. The following tests will pass in the library as it currently exists:

percent resultPct = sqrt(16.0_pct);
EXPECT_EQ(resultPct, 4.0_pct);
EXPECT_EQ(0.04, resultPct);

So the idea is that 4% when it's a dimensionless absolute value is 0.04.

@chiphogg
Copy link

chiphogg commented Dec 9, 2024

Shouldn't sqrt(16.0_pct) result in something like "4 perten" (made-up unit), which when converted to percent would result in 40.0_pct, not 4.0_pct?

@StefanoD
Copy link
Author

StefanoD commented Dec 9, 2024

If you go to my linked ticket, I showed different calculations which failed. This is IMHO a regression which previously worked in an older version.
Anyway: We are using the Aurora units lib now.

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 this pull request may close these issues.

3 participants