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

Custom units having unexpected effects on results #1306

Open
zcohan opened this issue Oct 30, 2018 · 8 comments
Open

Custom units having unexpected effects on results #1306

zcohan opened this issue Oct 30, 2018 · 8 comments

Comments

@zcohan
Copy link

zcohan commented Oct 30, 2018

I'm encountering an issue where custom units are changing how expressions that do not include the new unit are evaluated.

Example:

var result = math.eval('1 km / hour') //answer is 1 km/hour
math.createUnit('mph', {definition: '1.0 mile/hour'})
result = math.eval('1 km / hour') //answer is now 0.6213711922373341 mph
@ericman314
Copy link
Collaborator

That's a result of having the unit system set to the default of auto. A unit system defines the particular units results will be displayed in. In auto, the unit system is continually updated each time a unit is created or encountered. Not very functional in a programming sense, but it does help show the user what they probably want to see.

You can disable that using setUnitSystem('si') or similar.

@zcohan
Copy link
Author

zcohan commented Oct 30, 2018

Thanks @ericman314 , is that a function on the math object, or the unit itself? I can't find any mention of it on mathjs.org.

@ericman314
Copy link
Collaborator

I think you can use this:

math.type.Unit.setUnitSystem('si');

@zcohan
Copy link
Author

zcohan commented Oct 30, 2018

If I set the unit system only once after creating the math object, it doesn't work (I need to set it after creating the unit). So this works:

math.createUnit('mph', {definition: '1.0 mile/hour'})
math.type.Unit.setUnitSystem('si')
math.eval('(10 km/hour)') //answer is 10 km/hour

However, for my purposes the user will be able to add custom units at any time. Of course, I can just set the unit system again after any new unit is created, however it seems there is a bug: if I set unit system twice, it stops applying:

math.type.Unit.setUnitSystem('si')
math.createUnit('mph', {definition: '1.0 mile/hour'})
math.type.Unit.setUnitSystem('si')
math.eval('(10 km/hour)') //answer is 6.21 mph

@zcohan
Copy link
Author

zcohan commented Oct 30, 2018

Also, setting the unit system to 'si' undoes the fix you made the other day for the 'incorrect custom unit in unit multiplication result' issue:

math.type.Unit.setUnitSystem('si')
math.createUnit('USD')
math.createUnit('EUR', {definition: '1.15 USD'})
math.eval('10 EUR/hour * 2 hours') //23 USD, should be 20 EUR

Maybe auto can be improved such that 1 km / hour = 1 km/hour, even with custom units present? It doesn't seem intuitive to automatically convert into a custom unit, without the conversion being explicitly requested.

@ericman314
Copy link
Collaborator

ericman314 commented Nov 1, 2018

As for mph being used even after setting the unit system to si, I believe createUnit automatically adds the created unit to whatever the current unit system is.

As for EUR vs USD, the intelligent choosing of the unit to format the result in only works in the auto unit system.

It's hard to infer what the user wants to see. For instance, what should the result of 10 km/hour be if you had created the unit kph instead of mph? The current implementation does a good job most of the time. There's some clear improvements that could be made here though, and we might need to step back and rethink how formatting is done.

@zcohan
Copy link
Author

zcohan commented Nov 1, 2018

I definitely want the intelligent unit choosing behaviour, so I'll have to stick with auto.

Could we then consider 10 km/hour = 6.21 mph a bug with auto?

Personally I don't think it's a great design move to have the existence of custom units override the standard calculation behaviour. I might not be able to offer users the ability to add custom units at all, if as a result answers of unrelated calculations become unpredictably affected.

Furthermore, the (bug? feature?) seems to be only present for compound units:

math.createUnit('pickle', {definition: '5 km'})
math.eval('5 km') // 5 km, not 1 pickle, good!

math.createUnit('giffy', {definition: '5 km/minute'})
math.eval('5 km/minute') // 1.0 giffy, technically true, but not what I expect...

Would you agree that it makes more sense to explicitly request formatting in custom units?

math.eval('5 km/minute in km/minute') // This seems redundant, but it's the only way to avoid being giffied, now that giffies exist.
math.eval('5 km/minute in giffy') // 1.0 giffy, great.

@josdejong
Copy link
Owner

Thinking aloud: sounds like we want to somehow be able to give specific units some sort of precedence, i.e. give a custom unit like giffy lower precedence than SI units (km/minute). Or somehow have different categories or groups of units, and have a "barrier" to jump from one group (like SI units) to units outside of the group.

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

No branches or pull requests

3 participants