Flawed safe_speed calculations (classic jerk limitation)

[qx1147]

The safe_speed calculation in planner.cpp for classic jerk limiting seems to be flawed:

    const float nominal_speed = SQRT(block->nominal_speed_sqr);
    float safe_speed = nominal_speed;

    uint8_t limited = 0;
    LOOP_XYZE(i) {
      const float jerk = ABS(current_speed[i]), maxj = max_jerk[i];
      if (jerk > maxj) {
        if (limited) {
          const float mjerk = maxj * nominal_speed;
          if (jerk * safe_speed > mjerk) safe_speed = mjerk / jerk;
        }
        else {
          ++limited;
          safe_speed = maxj;
        }
      }
    }

Instead of safe_speed = maxj; (last line). it should be:

safe_speed = nominal_speed*maxj/jerk; or

safe_speed *= maxj/jerk;

My first thought was that the current code is correct only if current_speed[i] happens to be the only non-zero element in the speed vector because then jerk==current_speed[i]==nominal_speed (assuming the length of the current_speed vector is nominal_speed). Otherwise, at worst, safe_speed would just be assumed smaller than intended, so no big deal.

EDIT: But, on a second thought, this view might be too naive, because the elements of the current_speed vector might have different units and the concept of vector length doesn't make too much sense then; this is why everything is referred to some nominal speed in the first place. So I am completely unsure what the implications of this flaw really are. It might or might not explain problems like layer shifts or alike (e.g. https://github.com/MarlinFirmware/Marlin/issues/10446).

Personally, I did not experience problems which I could attribute to this flaw. But I haven't printed so much before and after the change, so I just cannot tell.

As far as I can see, this code snippet appeared in all Marlin versions around mid Dec 2017 (e.g. for bugfix-1.1.x, https://github.com/MarlinFirmware/Marlin/commit/477e36afab6f9e2cccf3856d6e689230cd27a177).

[AnHardt]

@thinkyhead That's interesting!

[Deneteus]

The next release was Dec 15' 2017 1.1.7 20f6a45 and the notes say adopted from Prusa Software.

I looked at the planner.cpp from November 11 2017 and the code looks like this:

  // Exit speed limited by a jerk to full halt of a previous last segment
  static float previous_safe_speed;

  float safe_speed = block->nominal_speed;
  uint8_t limited = 0;
  LOOP_XYZE(i) {
    const float jerk = FABS(current_speed[i]), maxj = max_jerk[i];
    if (jerk > maxj) {
      if (limited) {
        const float mjerk = maxj * block->nominal_speed;
        if (jerk * safe_speed > mjerk) safe_speed = mjerk / jerk;
      }
      else {
        ++limited;
        safe_speed = maxj;
      }
    }
  }

I couldn't find an explanation of the math behind the code change via Google Search. I would have expected some kind of explanation in the past/documentation and I did find this mention about Jerk issues though: #9917

Note: I am reading the documentation included in the CPP file.

[qx1147]

@Deneteus

I looked at the planner.cpp from November 11 2017 and the code looks like this:

You are right, so the code snippet was there already before mid Dec but disappeared for a few days. I should have paid more attention to the title of the commit I linked to ("Revert ...").

[AnHardt]

Current Prusa firmware still has about the same code - what not necessarily means it is correct there.

[alexyu132]

I've noticed that the latest Marlin with traditional jerk seems to corner slower than normal on my CoreXY machine with a lightweight MGN12 gantry - there's a lot of bulging on corners and curves, and the speed is heavily reduced, especially compared to junction deviation on Smoothie. The result is that curves look a lot less smooth than they should and the print itself takes longer. Higher jerk (tested up to 25) helps but doesn't alleviate the problem, and causes layer shifts if tuned too high.

Smoothieware on right, Marlin on left

Is this a limitation of traditional jerk, or is there a bug that's causing the corner speed to be limited too much? I don't remember seeing the same effect with Repetier-Firmware, though that was running on a different CoreXY machine.

[thinkyhead]

@alexyu132 — Probably better to compare Marlin with JUNCTION_DEVIATION enabled. Classic jerk limiting is bound to exhibit different effects.

[alexyu132]

@thinkyhead I haven't been able to get junction deviation working that well on Marlin, but I'll keep testing different values. However, Repetier uses traditional jerk limiting and doesn't seem to have the same issue.

[thinkyhead]

11396 applies the proposed change. Please check to see if it makes any difference.

[alexyu132]

@thinkyhead The change doesn't seem to make a difference on my machine. The traditional jerk behavior still seems to slow down more than normal, even with very high jerk values (tested up to 80 jerk), resulting in sharp speed changes that cause vibration and artifacts along curves at higher speeds. On my machine, increasing the jerk to about 20 makes it behave smoother.

Also, I was able to get junction deviation working by turning it down and it seems to work well now, though I haven't tested with honeycomb infill, which has been problematic in the past. It seems like the main issue with it is that it doesn't handle the case where the Z or E axes need to accelerate more slowly, as I was getting Z axis step loss during hops.

[thinkyhead]

It seems like the main issue with it is that it doesn't handle the case where the Z or E axes need to accelerate more slowly, as I was getting Z axis step loss during hops.

Interesting. We've been using the Prusa version of jerk limiting for quite a while now, and this hasn't been a widespread issue as far as we've seen. We did have to make sure users reduced the jerk values by half, since the previous algorithm was comparing against the halved value.

[alexyu132]

@thinkyhead the step loss was for junction deviation - traditional jerk is fine for step loss even at absurdly high values. One thing I'm wondering is whether the jerk and junction deviation values are in terms of the print head or the motors. Since my machine is a CoreXY it might explain the slower than normal cornering speeds.

Also, I've noticed that in Klipper there's a feature that caps the speed during movements too short to reach nominal speed. Something like this could make for smoother movements in curves rather than constantly being accelerating/decelerating between vertices and would be interesting to look into.

[qx1147]

I would guess that junction deviation should be interpreted more in terms of print head, because the algorithm appears to lump all axes together, including E though, which IMHO does not make sense at all. It seems to be odd already that one even cannot specify motor-specific jerk values anymore. But I really haven't looked into it, so it might be all garbage what I just said. Classic jerk, on the other hand, seems to be more about the motors, because the axes are treated independently from each other and each motor (or rather: each axis) can have its own jerk limit, which makes sense especially for setups where the axes are rather independent (e.g., head on X, bed on Y, Z on either one). But I think the whole jerk approach needs serious reconsideration, be it classic or junction deviation. Both algorithms don't seem to take into account the length and duration of the involved line segments, thereby allowing the effective jerk to build up far beyond the set limits. It is anyways unclear what jerk really means in this context. The algorithms kind of suggest an interpretation like: the motors can handle an abrupt speed change (=jerk) "once in a while", where the "while" is rather long in terms of motor and head motion dynamics (lines are treated as if they were of infinite length and duration). With short line segments, however, the "once in a while" becomes rather "very frequent", and the whole concept breaks down. Take, for example, a move that goes around a rounded 90° corner, lets say changing from from Y direction to X direction. The Y motor "sees" shorter and shorter segments during the move as it decelerates to standstill. If the segments are just short enough, the Y speed change from one segment to the next will always by smaller than the jerk, so jerk is not a limiting factor anymore for the print speed, and the print head is sent through the corner at full speed, while the Y motor has to decelerate to zero basically on the spot. I think that small segments will become more and more of a problem, because more people are using design tools which make e.g. edge filleting easy, and PCs have become increasingly capable of handling high-resolution models (time-wise and storage-wise), especially when it comes to slicing.

[AnHardt]

A few years back it was common knowledge: "High resolution models are unprintable." The fact stands - the knowledge got widely lost. Slicers able to limit segments to a minimal segment length helped with forgetting that. However, trusting blindly in that algorithms results mostly in worse results than constructions limiting the amount of polygons by will You will always get the bill for things you don't want to think about by your own.

[boelle]

@qx1147 is the problem still there with latest bugfix 2.0?

[qx1147]

@qx1147 is the problem still there with latest bugfix 2.0?

I don't know as I don't use Marlin anymore. Back then, with Marlin, and also now, with Repetier, I had/have to reduce jerk drastically in order to avoid layer shifts safely. But although jerk is a contributing factor, in my case the major problem seems to be caused by the Trinamic drivers when using StealthChop (TMC2130, so StealthChop v1). Apparently, StealthChop2 (e.g. TMC2208 or TMC2224) tries to fix these problems (http://forum.raise3d.com/viewtopic.php?f=4&t=6013#p24509) but, IMO, just might somewhat reduce chances of StealthChop failing in some edge cases, while increasing the risk of excessive motor currents in other edge cases.

[boelle]

@thinkyhead i think we can close this one

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.