Implementing auto Range for FixMath

[tomcombriat]

This implements what was discussed in #212 : promoting NI of FixMath only when needed.

The approach I am landing now is to keep track of the RANGE actually used by the number, initialized to the maximum holdable value by default.

This will probably make heavy use of Macro, because tests will be involved (not available in C++11 constexpr), but that should be a big deal as they are just template parameters which are not used for any calculation.

This draft PR is just to keep an eye on compilations and memory usage automatically.

[tomcombriat]

Okay, this seems to work! Only addition for UFixMath is done for now. It does not even seem to be so much trouble, so far. (Well, the compiler seems to produce quite some big lines to figure the RANGE but well, that's its job I guess!).

This version add the RANGE when performing an addition, and if the range exceed the biggest container, it promotes to the next one (but keep track of what is used in that one).

With that version:

UFixMath<8,0> a, b, c, d;
auto e = a+b+c+d;

correctly makes e a UFixMath<10,0> and not a UFixMath<11,0>.

Be patient for the rest, the lines do not turn that difficult, but I am trying to get my head not messing up the algebra too bad…

---

NB: It must be pointed that NI and RANGE are somehow redondant: the former is kinda contained in the latter. However, I am a bit reluctant to remove NI which I think would be more confusing for users than anything else. NI gives a clearer idea of the number of bits used, which is important for optimizations, and have the same "unit" than NF. This is especially true as I intend RANGE to be hidden. Other opinions welcomed though!

[tomcombriat]

Ok, slowly getting there… Not as straightforward as I initially thought (you can have a look at the MACRO I had to use…). Nearly everything is done, except the multiplication and opposite for SFix.

One of the small difficulties funnily (maybe for me only ;) ) comes from the fact that we are using two's complement for negative numbers. It follows that the attainable range is not exactly the same for unsigned (255 in 8bits) and signed (256 in 8 coding bits). But I took that into account, so for instance, the library knows that, in the following example, c can only go to 127*2=254 and not 255.

UFixMath<8,0> a, b;
auto c = a+b; 

The gain is not very impressive on one addition but that can allow to squeeze one more small number in some cases, after a few addition.

---

Will try to finish, clean up and have a look with a fresh eye soon (a quick glance from someone else might be helpful there too), I discovered some errors I did in the previous implementation already (especially in the "smart" shift, which should be careful if NI or NF goes to 0 because of the shift)…

---

PS: do not understand why the reports do not work, I started from the devel/FixMath2 branch which was up-to-date with Github actions…

---

PPS: If this works, I am planning to remove all non-safe operators, that were initially intended for fine tuning the resulting range as they won't be needed anymore I think. Fine-tuning will be achieved with:

• correct initial choice of the types
• casting between SFix

The only non-safe behavior will be construction from integers and casting between SFix, but I am very reluctant to remove them as the former quite ease the syntax, and the latter allows fine-tuning. Functions .toSFraction and toSInt (and their respective U counterpart) will be privileged in examples.

I guess, I'll just comment them out via a pre-processor condition (which could possibly be changed by the user at the beginning of their sketch to activate the "extended" set of possible operations).

[tfry-git]

I looked at your changes, and could not find anything wrong, but it's quite complex stuff, indeed.

Regarding the NEEDEDNIEXTRA macro, I wonder, whether NI is actually needed as a parameter, here? Isn't it rather used as "NIFORRANGE", essentially?

At any rate, I would highly welcome some automated tests. One way to implement those might be as follows (directly in FixMath.h):

namespace MozziPrivate {
void testUFix() {  // Just a dummy. This function will never be called.
  const UFixMath<7, 0> a = 66;    // as long as these are declared const, we can use them in static_assert()s
  const UFixMath<0, 1> b = .5;

  static_assert((a*b).SR(1).asRaw() == 33);
  static_assert((a+b).getNI() == 7);
  static_assert((a+a+a).getNI() == 8);
  // many more tests to be added, here, over time
  // both to document expected behavior, and to make sure future changes don't break it
}
}

(While typing the above, I was looking for an asInt()-function - perhaps we decided against that in order to avoid confusion with asRaw(), but how about an integerPart() getter, perhaps.)

--

No idea, why reports don't work. Perhaps comparison across forks is an issue, here?

--

Regarding the plans, hiding away the unsafe functions, until explicitly requested, sounds good to me.

Yes, we'll need casting within the S/UFix types. I still think it might be possible to come up with a set of member functions that makes some such casts easier to read - if designed, properly. (You remember my half-baked idea on clip() and trim(); floor() (shift out by NF) is another one that I imagine could be useful).

As for correct initial choice of types, it would be cool if we could somehow coerce the compiler to apply a proper RANGE, when initializing from a constant. Not sure, if that can be done.

[tomcombriat]

Thanks for the look!

Regarding the NEEDEDNIEXTRA macro, I wonder, whether NI is actually needed as a parameter, here? Isn't it rather used as "NIFORRANGE", essentially?

I am not sure I got that, is it just the name? (Because NI is definitively needed in that macro).

---

I would have never though of that way to test things, clever! Will try to add some as I go (I am testing all the cases I expect to be problematic while implementing so I have a quite good idea of what can pose problem now).

Yes, all these functions could be nice but so far I could not think of a case where they would be needed in Mozzi. I suggest keeping that on hold for the time being and see how the examples develop (once I manage to get there…).

---

As for correct initial choice of types, it would be cool if we could somehow coerce the compiler to apply a proper RANGE, when initializing from a constant. Not sure, if that can be done.

Yeah, that would be quite nice. Honestly if we could find NI automatically I would consider myself happy already! A lot of the mess at the beginning of the source code results from me trying. Too bad macros cannot be recursive! Of course there is always the possibility to make a gigantic macro that would test all 64 possible cases for NI.

For all this, I am also wondering if hiding as much as possible NI and NF is always a good idea. I think it is also nice for users to be aware and get trained of the things that are being manipulated, at least to an extend and not always rely blindly on the capacities of FixMath, which allow to implement more advanced stuff. I might be a bit conservative on that though. The fact that NI cannot be inferred with certainty by the programmer using FixMath because of RANGE is already a bit weird IMO.

[tfry-git]

Except of course, my static_asserts don't seem to compile. I'll look into how this is (hopefully) possible.

[tfry-git]

Ignore my comment regarding NEEDEDNIEXTRA: I somehow took NI+NF as a given, and then concluded, NI was redundant...

---

Too bad macros cannot be recursive

Templates can ;-)

template<int64_t value, int8_t bits=0> class BitCounter {
public:
    static constexpr char countbits() {
       return((1LL << bits) > value ? bits : (BitCounter<value, bits+1>::countbits()));
   }
};
// partial specialization to stop counting at 64
template<int64_t value> class BitCounter<value, 64> {
public:
    static constexpr char countbits() { return 64; }
};

static_assert(BitCounter<127>::countbits() == 7);

---

Regarding my static_asserts: These can be made to work by making a lot of stuff constexpr.

• For a lot of the functions that may actually be useful in its own right.
• The c'tor would also to be constexpr, which (in c++11) means no function body allowed at all:

  template<typename T>
  constexpr UFixMath(T value, bool as_raw) : internal_value(as_raw ? internal_type(value) << NF : value) {}

  • Where it gets a bit nasty is the + and * operators, however. Again, C++11 strictly insists on "only a return statement", which is quite possible, but at the cost of readability.
  • Finally, the compiler also seems to need a few extra "constexpr" thrown in in the test code itself (e.g. const auto constexpr a = UFixMath<7, 0>::fromRaw(66), for whatever reason.

So, all in all, this is again more complex than expected (even if I tend to think it may be worth it in the long run), and I suggest to ignore it for the moment.

[tomcombriat]

Alright, I think it looks quite close to a final first step.

The unsafe functions are deactivated by default, they can be activated by #define FIXMATHUNSAFE before including FixMath.h (I am quite sure you will come with a cleaner solution).

Remains:

• [ ] Documentation
• [ ] Adding toSFix for UFix and the opposite: safe casting to the other type which also keeps the RANGE (which I think is not possible with the standard cast defined in constructors).

---

PS: github says the only example I modified does not compile anymore… It compiles here though, will have a look a it…

[tomcombriat]

Templates can ;-)

Good catch! I add that to my ever-growing list!

---

The c'tor would also to be constexpr,

Is that possible even if the arguments taken by the constructor are not const (like it would happen when say, casting from any changeable value like mozziAnalogRead())? Or two versions would be needed?

Also, on readability, the places mixed operators are defined follows somehow a logic (in UFix except if the code is more similar to SFix) but I could agree it is not straightforward, especially as their respective opposite are defined as free functions at the end to avoid code duplication (they can be defined by calling the functions with reversed operators). All mixed comparisons are together at the end somehow, but I found it quite clean there because they are quite simple, compared to the operators…

[tfry-git]

Templates can ;-)

Good catch! I add that to my ever-growing list!

Note that I'm not sure, how this would be applied towards detection of NI, however, esp. in light of the following:

The c'tor would also to be constexpr,

Is that possible even if the arguments taken by the constructor are not const (like it would happen when say, casting from any changeable value like mozziAnalogRead())? Or two versions would be needed?

constexpr is just a promise that the statement can be evaluated at compile-time, if all relevant parameters are known at compile time. It may be helpful to know that there is also a - different - consteval (C++20 or so, far out of reach), which enforces that a function may only be evaluated at compile-time.

So one constructor is still enough.

Also, the BitCounter-template could also be made to work on a variable input, but it would produce terribly inefficient code in that case.

[tomcombriat]

(Replaced all the macro with constexpr, now that I know ternary are allowed there… I hope to get the extra point ;) ). Let me know if changing some of them was actually not clever.

Cleanup here and documentation in #212 (once merged) still needed.