Config changes in unitmath

[ericman314]

One of the outstanding items in the Unit.js refactoring is in config changes. I'm looking for some direction and clarification on how to proceed. Mostly I am just finding myself a little behind in understanding all the changes made in v6.

1. Am I right in observing that v6 changed the way that a user sets the math.js config?
2. Are there now two separate methods used to change the configuration, math.config and math.create?
3. Does every math.js instance have an immutable configuration? Or is only the default instance immutable?
4. In Unit.js, there is an event handler for math.js config changes. If the math.js config mutates, we would be required to create a new instance of unitmath since the configurations in those instances are immutable. What would be the best way to go about doing that?
5. If a new instance of math.js is created, am I right in assuming that createUnitFunction is run again? In that case, it would be necessary to read the config when creating the unitmath instance. What is the correct way to do that?
6. Whenever createUnit is called, we will need to create a new instance of unitmath. In the new library, the unit definitions are immutable. Should we handle this the same way we would handle config changes?

[josdejong]

heee you have a new avatar, where did the white helmet go ;)

In v6 the challenge was that I wanted to turn all functions into standalone, pure functions. This is supported now, though we still needed a way to listen for config changes in some cases, like BigNumber and Unit. I'm not fully happy with the hybrid solution solution, though at least we now have the possibility to have standalone functions. The underlying mechanism is roughly the same as in mathjs v5: You can still subscribe to listen for changes in config using the dependency on, though all functions have this optional now, so you can also create them as a real, pure function.

Basically, you can pass 'config' to a factory function to get the initial config, and if you want, create an dependency 'on' or an optional dependency '?on' to listen for future changes, like for example Unit.js does.

1. At the core, the old mechanism is still there, though it is optional now, allowing for standalone functions
2. You can pass initial config via create, and change the config later via math.config. Passing config via create is optional. It allows creating "pure" functions immediately by destructuring the returned mathjs instance directly, ignoring the math.config function that is also returned:

   const { sqrt } = create({ createSqrt }, config)

3. In the default instance, the config function is replaced with a readonly version, and the event listener on is not defined. That makes the exported functions pure and immutable. Any instance created via create is not immutable, though if you never touch the config function it will never change (see 2).
4. I think the factory function should create a mathunit instance and keep it in a variable let mathunit = .... When the config changes (on('config', ...)), replace the mathunit instance.
5. Correct, when creating a new mathjs instance, the factory functions are invoked again (you can try it by putting a console.log in the factory function). It's really isolated from other mathjs instances. On creation, you should use the config that is passed via the dependencies.
6. I'm not sure what you mean here. Indeed it will need to use the passed config on creation, and after that listen for config changes via on('config', ...). Since mathunit is immutable, it could be possible that you do not need to create a new mathunit instance but reuse a single instance. But maybe start simple and optimize this in a later stage?

I'm really looking forward to have MathUnit integrated, there are more and more issues that will be solved by MathUnit, it really looks like the way to go 😎

[ericman314]

Thanks for the clear explanation. I'll start playing around with it.

[ericman314]

Another thought I had: Would it be possible, or even a good idea, to divide the Unit type into four sub-types, like Unit_Number, Unit_BigNumber, Unit_Fraction, and Unit_Complex? Each would have its own UnitMath instance. Some advantages I can think of are that typed_function would automatically select the correct instance for operations. But it seems like there would be four times as many function signatures as there would be for just a single Unit type. Could Unit become an abstract type??

[josdejong]

That's an interesting thought. Each of the four implementations will be relatively easy and straight forward to implement. But the downside is that you have to keep four implementations in sync with each other, and there is no way to use your own favourite BigNumber library. If these four implementations have hardly anything in common qua implementation, this makes sense. I have the feeling though that they do have a lot of logic in common, and that the current solution where the actual numeric type and numeric operations are abstracted works nicely. A small experiment to explore your idea can be fun and informative though, it can result in new insights :).

[harrysarson]

Unit<T> 😮

[ericman314]

Finally got the definitions for degrees and so forth updating correctly when the precision config option changes. Turns out one of the typed function signatures was using the original configuration instead of the new configuration.

Whereas the buggy code used the original unitmath instance:

'string': unitmath()

The correct code uses the current instance:

'string': (...args) => unitmath()(...args),

It's these kinds of bugs that are so difficult to track down which is why I don't particularly like how the interface is shaping up. The UnitMath library is very pure and clean thanks to the many suggestions from @harrysarson. And now in v6, math.js has an amazing dependency injection mechanism. It's just been tricky to get the two to work together. But if we can get it working, we can look at improving the glue between the two libraries later.

Next task is to tackle the createUnit function.

[josdejong]

Ah, that makes sense. Good to here you're making progress 👍

Yeah, well it's mostly mathjs to blame. mathjs now is an unideal hybrid between pure functions and a stateful instance. Let's make sure that UnitMath itself stays neat clean :)

[ericman314]

I'm starting work on createUnit. Another question: when math.create is used to create a new instance, is anything besides the config options inherited? If I remember correctly, the entire unit factory is re-run when doing math.create, so that would clobber any existing custom units. Therefore, I'm not sure how I would make the following work:

math.createUnit('foo')
const math2 = math.create()
// math2 should contain foo, but how?

Of course, if we could do things over I would suggest making custom units a part of the config options, and getting rid of the stateful createUnit.

Another option is we could make custom units part of the config, and just make createUnit a wrapper around altering the config, for backwards-compatibility.

[josdejong]

That's good to hear!

const math2 = math.create() is will not create a full clone of your current mathjs instance. It will create a completely new instance with default config. It will not contain any functions you imported using math.import(...), and I think it should also not contain custom units created in the initial instance. I think that should make life easy for unitMath, right?

[ericman314]

Oh ok. Then yes that will be much easier!

…

On Sun, Aug 4, 2019, 3:17 AM Jos de Jong ***@***.***> wrote: That's good to hear! const math2 = math.create() is will not create a full clone of your current mathjs instance. It will create a completely new instance with default config. It will not contain any functions you imported using math.import(...), and I think it should also not contain custom units created in the initial instance. I think that should make life easy for unitMath, right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1551?email_source=notifications&email_token=ABQNHEIWUFJVAESBG3BZEPDQC2NDPA5CNFSM4H4JLCBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3P5WMI#issuecomment-517987121>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQNHEMQLA5DIND4BRWVQTDQC2NDPANCNFSM4H4JLCBA> .

[ericman314]

All right, got createUnit mostly done. Only 20 failing tests remaining (down from 200 originally)!

Question about unit.setUnitSystem. This is yet another example of a function that could just as easily be made an option in the math.js config. I haven't yet invested much time implementing or testing unit systems in UnitMath itself, so we can be flexible here. What do you think?

[josdejong]

Woohoo! That's good news 👍

Configuring unit.setUnitSystem via the mathjs config sounds good. I think it's quite a core setting for UnitMath, right? It's in a sense similar to the config option precision, which is a core setting for BigNumber.

[josdejong]

On a side note: in some future version of mathjs (not too soon) I would like to see if we can fully move away from mathjs instances. Currently we have a complicated hybrid between pure functions and a stateful instance. Not sure if feasible and how complicated it will be, I would have to do some experimentation. The simplicity and beautiful API of UnitMath really inspires me in this regard 😄 .

[josdejong]

@ericman314 just curious: what is the latest status of UnitMath? Are you still working on it?

[ericman314]

I'm planning to work on it when I have time, so, hopefully soon. What's your timeline for v7?

[josdejong]

Mathjs is on the back-burner from my side these months, I'm working on an other project (jsoneditor). So I think v7 will only contain the new unitmath library, which is huge by itself so totally worth a major release :)

[m93a]

Sorry, that was a misclick 😅️

[ericman314]

UnitMath is now v1.0.0-rc.1, please check it out! https://www.npmjs.com/package/unitmath/v/1.0.0-rc.1

[josdejong]

Awesome to hear that v1 is close 😎 . I'll have a look soon Eric.

[josdejong]

It's awesome @ericman314 . I left a few minor comments in the repo.