Accumulating errors in flow rate calculations

[qx1147]

The way in which the flow rate is currently taken into account, round-off errors accumulate and cause under-extrusion. This problem becomes quite relevant if segments are very small and extrusion volume (layer height x width) is low, i.e., if there are only a few extruder motor steps to be made per segment. With my printer, a Cartesian with direct drive gear, the extruder motor makes just 5 micro-steps per mm XY travel (1.75mm filament, 0.2mm layer height, 0.4mm extrusion width), so there is no room for unnecessary round-off errors, let alone accumulating errors leading to a wrong flow rate.

I suggest to change
p->delta[E_AXIS]=(long)((p->delta[E_AXIS]_(float)Printer::extrudeMultiply)0.01f);
to
{
p->delta[E_AXIS]=(long)((p->delta[E_AXIS](float)Printer::extrudeMultiply)_0.01f + 0.5f);
Printer::destinationSteps[E_AXIS] = Printer::currentPositionSteps[E_AXIS] + p->delta[E_AXIS];
}

This or similar changes have to be made wherever extrudeMultiply is applied.
I have not tried this yet and I am not sure about the side effects of adjusting destinationSteps in this way, but adjusting destinationSteps in addition to just rounding would cause the round-off error to be propagated to and be taken into account in the next segment, which is better than just waiting for the errors to average out on the long run.
This is still a sub-optimal solution due to the repeated integer conversion, once when destinationSteps is calculated in the first place (setDestinationStepsFromGCode) and once when applying extrudeMultiply, so there is still room for improvement beyond the suggested quick fix.

[repetier]

That solution might be better but could also produce too much filament. I think the correct solution would be to add the last error as float and set the error to the new rounding error. That way the errors add until it is enough for a full step. Not a half step error per segment. But that of course only matters for very small moves.

[qx1147]

Oops, you're absolutely right. My solution of propagating the error is plain wrong because it obviously also propagates the change induced by extrudeMultiply (destinationSteps and currentPositionSteps must stay related to the original scale). So yes, using an explicit error variable is the way to go.

[repetier]

with float Printer::extrudeMultiplyError = 0 initialized it should look like this:

    if(axis == E_AXIS && Printer::extrudeMultiply!=100) {
        Printer::extrudeMultiplyError += ((axis_diff[E_AXIS] * (float)Printer::extrudeMultiply) * 0.01f);
        axis_diff[E_AXIS] = static_cast<int32_t>(Printer::extrudeMultiplyError);
        Printer::extrudeMultiplyError -= axis_diff[E_AXIS];
    }

I still need to test this version, but I'm quite confident that it improves the quality with flow multiply.

[qx1147]

Yes, that looks good. I cannot see here, however, whether the axis_diff[E_AXIS] is already unsigned in your code snippet. If so, I would suggest to consider applying the correction earlier, when axis_diff[E_AXIS](or whatever delta variable) is still signed - for the sake of correctness (would also avoid unnecessary inaccuracies during retraction, depending on where and how retraction is handled).

[repetier]

Right, except from my solution using the wrong array it was after making positions absolute. Will correct and test it.

[qx1147]

Bug in motion.cpp, line 176 (work092-branch):

Printer::extrudeMultiplyError -= axis_diff[E_AXIS];

should be (I guess):

Printer::extrudeMultiplyError -= p->delta[E_AXIS];

axis_diff[] is not even defined before being used here.
I did not test this as I still use 0.91, but I heard of problems with 0.92 which could be well explained by this bug. This could also help solving issue #321.

[repetier]

@qx1147 Thanks. That is exactly my solution, which I programmed 2 times correct and at one position wrong (the one you found). Since I normally test with my delta I got the correct versions, while cartesian printers got the wrong one. Will update soon, after some more tests.