Skip to content

Feature: Power#44

Merged
angularsen merged 2 commits into
angularsen:developfrom
bretthysuik:feature/power
Nov 7, 2014
Merged

Feature: Power#44
angularsen merged 2 commits into
angularsen:developfrom
bretthysuik:feature/power

Conversation

@bretthysuik
Copy link
Copy Markdown
Contributor

Power has a very wide range of usable values (examples) so I'm not sure where exactly we'd want to draw the line.

There's also a tolerance issue that came up in the femtowatts unit tests: using double to store femtowatts leads to rounding/representation issues.

protected override double FemtowattsTolerance
{
    get { return 0.15; } // Need to use 0.15 instead of the standard 1e-5
}

I was thinking about this previously and I think decimal might be a better backing datatype because it can exactly represent powers of 10 because it's a decimal floating point type (double is binary floating point). It would make sense to use it since we're mainly concerned with SI units which are powers of 10, however it's a major breaking change and decimal is less efficient for calculations...

@angularsen
Copy link
Copy Markdown
Owner

In case you haven't noticed there is already support for double, long and decimal backing types. See Information unit where I had similar issues for exabytes etc. Would that work?

@bretthysuik
Copy link
Copy Markdown
Contributor Author

Ah, I never noticed the "BaseType" attribute in the json files. That should work for power.

@bretthysuik
Copy link
Copy Markdown
Contributor Author

Is there anything else in the pull request that needs to be addressed?

angularsen added a commit that referenced this pull request Nov 7, 2014
@angularsen angularsen merged commit d9a5319 into angularsen:develop Nov 7, 2014
@angularsen
Copy link
Copy Markdown
Owner

No looks good. Thanks.

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.

2 participants