Skip to content

Decompose unit conversion functions to exact coefficients (breaking) - #1499

Merged
angularsen merged 5 commits into
angularsen:release/v6from
lipchev:fix-unit-conversions
Apr 6, 2025
Merged

Decompose unit conversion functions to exact coefficients (breaking)#1499
angularsen merged 5 commits into
angularsen:release/v6from
lipchev:fix-unit-conversions

Conversation

@lipchev

Copy link
Copy Markdown
Collaborator

This is a port of #1393, targeting the release-v6 consisting of the changes that broke one test or another.

Most of these are related to conversions involving the US/British units but there is also some confusion regarding the Calorie - the value of 4.1868 was used (instead of 4.184) in a few places, which is a note-worthy difference.

Here are all the tests that broke:

@lipchev

Copy link
Copy Markdown
CollaboratorAuthor

I still want to replace the PressureUnit.FeetOfElevation and PressureUnit.MeterOfElevation with properties that convert to/from Length (see #1393 (comment)) and remove the Frequency.BUnit but I'll open separate PRs for these (didn't want to have the Units.g.json changed here).

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@angularsenangularsen changed the title Decomposed the rounded expressions in the unit definitions into operations with exact coefficients (breaking)Decompose unit conversion functions to exact coefficients (breaking)Mar 23, 2025
@angularsen
angularsenforce-pushed the fix-unit-conversions branch from 5da8e22 to 80d3692CompareMarch 23, 2025 17:16
Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/HeatFlux.json

@angularsenangularsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not through everything yet but posting the review so far.
I got through all the JSON files.

Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/HeatFlux.json
Comment threadCommon/UnitDefinitions/SpecificFuelConsumption.json Outdated
Comment threadCommon/UnitDefinitions/ThermalInsulance.json
Comment threadCommon/UnitDefinitions/ThermalInsulance.json
Comment threadCommon/UnitDefinitions/VolumetricHeatCapacity.json
Comment threadCommon/UnitDefinitions/VolumetricHeatCapacity.json
…umetricHeatCapacity and Power
- HydraulicHorsepower: conversion coefficient changed from {x} * 745.69988145 to 745.69987158227022
@lipchev
lipchev requested a review from angularsenMarch 24, 2025 07:42
@lipchev

Copy link
Copy Markdown
CollaboratorAuthor

@angularsen I've merged the modifications from release/v6, this should be ready to review- please tell me if there is something else you want me to do.

I'd really like to close this as soon as possible- it's blocking my 🐲 PR..

@angularsen

Copy link
Copy Markdown
Owner

I'll make an honest attempt the next few days, I know I'm running late on a bunch of PRs lately and it's on my conscience, I just struggle to find the time and energy.

@angularsen
angularsen merged commit 5a280b9 into angularsen:release/v6Apr 6, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@lipchev@angularsen