feat(units): Add Ah unit

[adrfantini]

This PR adds Ah, the ampere-hour, as a unit of electric charge.
In the energy and technology, this unit is used all the time much more often than A h or A⋅h.

The unit of measure Wh is already considered for energy, so this is in line with that line of thought.

[gwhitney]

This looks pretty straightforward to me, but I defer to @josdejong on approving any Unit-related PRs because of a long-standing desire to make Unit an independent package and never being quite sure how changes play into that. However, if it is generally approved as to the content of the change, this PR will need some additional work: there would need to be an addition to docs/datatypes/units.md, and at least one test added for the new Amp-hour Ah unit.

[adrfantini]

This looks pretty straightforward to me, but I defer to @josdejong on approving any Unit-related PRs because of a long-standing desire to make Unit an independent package and never being quite sure how changes play into that. However, if it is genarally approved as to the content of the change, this PR will need some additional work: there would need to be an addition to docs/datatypes/units.md, and at least one test added for the new Amp-hour Ah unit.

Hi and merry Christmas! 🎄

I added two commits, one modifying units.md and one adding a test for Ah.
I am not sure if that was what was requested though, could you verify before I do the same for the other Merge Requests? Thanks in advance!

[gwhitney]

I am not sure if that was what was requested though, could you verify before I do the same for the other Merge Requests? Thanks in advance!

Yes, that's all good. Also, in this particular case since it's a new compound unit, you should test that an amp times an hour is an Ah and that sort of thing. I think but am not sure that such products, unless they have automatic simplification off, should now just end up as Ah when computed. In any case, it would be good to test whether that's true, and then we can make sure that it is working the way Jos intends, once he has a chance to do the final review of this. Thanks so much!

[adrfantini]

I'm having some difficulty here.

The new Ah unit is defined using Wh as template. In fact, they are very much alike:

    Ah: {
      name: 'Ah',
      base: BASE_UNITS.ELECTRIC_CHARGE,
      prefixes: PREFIXES.SHORT,
      value: 3600,
      offset: 0
    },
    Wh: {
      name: 'Wh',
      base: BASE_UNITS.ENERGY,
      prefixes: PREFIXES.SHORT,
      value: 3600,
      offset: 0
    },

However, I tried adding a couple of tests and the one for Ah passes, while the one for Wh fails:

    it('should be able to combine A and h into Ah', function () { // PASSES
      const n1 = 2
      const n2 = 3
      const unit1 = new Unit(n1, 'A')
      const unit2 = new Unit(n2, 'h')
      const unitM = unit1.multiply(unit2).simplify()
      assert.strictEqual(unitM.units[0].unit.name, 'Ah')
      assert.strictEqual(unitM.value, n1 * n2 * 3600)
    })

    it('should be able to combine W and h into Wh', function () { // FAILS
      const n1 = 2
      const n2 = 3
      const unit1 = new Unit(n1, 'W')
      const unit2 = new Unit(n2, 'h')
      const unitM = unit1.multiply(unit2).simplify()
      assert.strictEqual(unitM.units[0].unit.name, 'Wh')
      assert.strictEqual(unitM.value, n1 * n2 * 3600)
    })

The error is:

  1) Unit
       multiply, divide, and pow
         should be able to combine W and h into Wh:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

'J' !== 'Wh'

      + expected - actual

      -J
      +Wh

      at Context.<anonymous> (file:///home/adriano/Documents/nextcloud/current/sw/mathjs/test/unit-tests/type/unit/Unit.test.js:1096:14)
      at process.processImmediate (node:internal/timers:504:21)

Why does mathjs simplify Wh into J but not Ah into C? Maybe because A is a base quantity while W is J/s?

Another thing that confuses me is that it appears that multiply should return a unit that automatically simplifies (skipAutomaticSimplification === true), but still simplify() changes the unit! So is it simplified or not?

At this point I'm not exactly sure what is the expected behaviour and thus what I should test here.

[adrfantini]

I'm having some difficulty here.

...

Ah! It dawned on me the reason for this! The default unit system has J as unit for ENERGY, so it prefers to convert to that.
I added a Wh test anyway, just to clarify and verify.

Let me know if this is all right and then I'll go on with the other MRs!

[gwhitney]

All looks good now and thanks for including the Wh test, too. But now the name of the test is a bit misleading; please change the name of the test to "Should be able to combine W and h into J", thanks. Since that's a completely trivial change, I am authorizing the CI checks and as I said since this is a Unit PR, @josdejong will have to take the review from here and land this PR as he sees fit. Thanks for the contribution!

[adrfantini]

All looks good now and thanks for including the Wh test, too. But now the name of the test is a bit misleading; please change the name of the test to "Should be able to combine W and h into J", thanks. Since that's a completely trivial change, I am authorizing the CI checks and as I said since this is a Unit PR, @josdejong will have to take the review from here and land this PR as he sees fit. Thanks for the contribution!

Sure thing! Ready now; I'll wait for @josdejong's review before updating the other MRs as well

Thank you!