Replace usage of approxEqual with isClose

[Geod24]

And a couple minor commits for things laying around.

[Geod24]

dstats ~master: building configuration "dstats-test-library"...
source/dstats/base.d:814:23: error: template ‘std.math.operations.approxEqual(T, U, V)(T value, U reference, V maxRelDiff = 1.0e-2, V maxAbsDiff = 1.0e-5)’ is deprecated - approxEqual will be removed in 2.106.0. Please use isClose instead. [-Werror=deprecated]
  814 |     assert(approxEqual(auroc([8,6,7,5,3,0,9], [3,6,2,4,3,6]), 0.6904762));
      |                       ^

What the heck ? Why is -de enabled by default ? :|

[Geod24]

@John-Colvin : This is ready to go

[John-Colvin]

There's quite a lot of orthogonal changes here which could easily go in on their own, they are already cleanly separated as separate commits.

[John-Colvin]

This introduces a lot of magic 1e-2 everywhere, this needs some explanation in the code which probably means it needs a wrapper function.

Also, some uses of isClose you have introduced don't have the extra 1e-2 arguments.

The tests should remain quite "tight" numerically speaking.

[Geod24]

I can raise a separate PR with those commits if you prefer.

This introduces a lot of magic 1e-2 everywhere, this needs some explanation in the code which probably means it needs a wrapper function.

Also, some uses of isClose you have introduced don't have the extra 1e-2 arguments.

The tests should remain quite "tight" numerically speaking.

That's what the deprecation note recommends. I originally added an approxEqual locally, but figured it would just be painting over the problem. After the massive changes, I tried to go in and remove some of the 1e-2 arguments, but in many cases, I failed. My understanding is that isClose is actually stricter than approxEqual, but I'd prefer someone more familiar with the code have a look themselves.

[Geod24]

@John-Colvin : How do you prefer I split this ? Last commit can go in its own PR already, it's one line.
Second commit depends on the first, so if the first has to go in its own PR, I'll take it out temporarily.

Regarding isClose / approxEqual: Just introducing an approxEqual that does (almost) the same thing as isClose would be trivial but also not very useful. I wanted to make it easy for people more familiar with stats to adjust the tests in the future, when the explicit arguments. I removed some myself but given the amount of lines, it would be daunting to go over everything.

[John-Colvin]

IMO one way to look at how to split things is: "anything that is sensible to be reviewed separately, should be submitted separately". "Sensible" does some heavy lifting there, obviously in theory every of these isClose changes are independent, but it makes much more sense to at least try to review them in large bundles and see if there's a problem and break up if there is e.g. "these are all sensible but please change this one". That would be a point to stop and go "could I have seen this coming and split those our already". But the other change, the allocator one, has no relationship to the others so we shouldn't couple them.

Essentially the whole point is to reduce coupling without causing decoherence.

[Geod24]

Moved one of the commits to #49 , and took out the code removal as it's not strictly needed for this.

@John-Colvin : The issue with the remaining giant commit is that I can't split it: isClose didn't exists before gdc-12, and gdc-12 treats deprecations as error.

[Geod24]

As can be seen from the CI, actually

[Geod24]

Rebased

[John-Colvin]

@maxhaton can you take a look at this from a numerics point of view? isClose with the 1e-2s and is that likely to matter here vs the old approxEqual?

[Geod24]

Ping @maxhaton

[maxhaton]

Ping @maxhaton

The changes are probably fine but being pedantic the chosen value for the max absolute difference is quite a bit more lenient than what is implied by approxEqual originally I think.

[Geod24]

@maxhaton : I simply followed https://dlang.org/changelog/2.091.0.html#isClose

[maxhaton]

@maxhaton : I simply followed https://dlang.org/changelog/2.091.0.html#isClose

I think it should be 1e-5 instead - the changelog does not agree with the deprecation message and docs. Let's get this merged

[John-Colvin]

@maxhaton : I simply followed https://dlang.org/changelog/2.091.0.html#isClose

I think it should be 1e-5 instead - the changelog does not agree with the deprecation message and docs. Let's get this merged

Let's get it right. If it should be 1e-5 then it should be 1e-5, I don't want to make all the tests massively more lenient.

[maxhaton]

Ok.