Skip to content

Conversation

@dmbutyugin
Copy link
Collaborator

This PR substantially reworks how the automatic shaper selection processes multiple resonance data files. Some printers can have different resonances at different points of the build volume (e.g. bed slingers at different Y positions, some CoreXY printers when gantry or Y rods have insufficient stiffness, delta printers across build volume in general, etc.), so in order to do a proper input shaper selection, one may need to run resonance testing at different points, and somehow combine the results. The current Klipper script just combines the resonances data from different files and averages the measurements for the same frequencies. This has a downside that individual resonance peaks at specific points may be smoothened out by data at other points, sometimes leading to poor input shaper selection. The new reworked algorithm instead estimates remaining vibrations (as a percentage) for each shaper for each resonance measurement individually, and then selects the input shapers that minimizes the vibrations across all measurements. In addition, this PR brings a couple of other minor changes to the shaper selection algorithm:

  • No longer recommend ZV shaper by default if possible - this input shaper shows relatively poor performance in practice due to high sensitivity to the shaper frequency and damping ratio selection, which makes it a poor choice for an automatic recommendation (the data is still presented for ZV shaper and a user can make an informed choice to enable it if they choose so).
  • An additional pass over the shaper frequency selection to try to improve over the previous best shaper; this is useful on occasions like when MZV was selected with good smoothing results but relatively large remaining vibrations, and the best 2HUMP_EI happens to give too much smoothing and thus is not selected, but it could be possible to select a different 2HUMP_EI shaper frequency that gives both smaller remaining vibrations and smaller smoothing than MZV, making it a better recommendation.
  • When the minimal estimated shaper vibrations are exactly 0, permit a very small non-zero vibrations value for the 'best' shaper recommendation.

Here's a comparison for a few cases of the current and the new algorithm for multi-point automatic input shaper selection:

Current algorithm New algorithm
old_x_resonances new_x_resonances
old_y_calibration new_y_calibration
old_z_calibration new_z_calibration
old_y_calibration new_y_calibration

And while you can see that the input shaper recommendations are not always different, sometimes it is more accidental, and the new algorithm still better accounts for individually measured resonances.

* Do not recommend ZV shaper by default if possible
* Try to find more optimal shaper out of more aggressive ones

Signed-off-by: Dmitry Butyugin <[email protected]>
@KevinOConnor
Copy link
Collaborator

Thanks. In general it seems interesting and useful to me.

One question I have - would it be reasonable to convert from the current /tmp/xxx files to the webhooks api for this data? That is, instead of changing the existing file format could we instead export the data over the webhooks unix domain socket, and change the graph_accelerometer.py and calibrate_shaper.py scripts to read from the socket? There are a few benefits to doing that: I'm sure the gui frontends would love to also have access to this data; it would avoid having to warn about changing file formats in the future; it could avoid the need to ssh/scp the graphs to another machine for display purposes. For examples of code that does this today, one can look at how klippy/extras/bed_mesh.py implements the "bed_mesh/dump_mesh" endpoint and how scripts/graph_mesh.py can read that data from the socket (or from moonraker or from a file produced from a previous invocation of the script).

Otherwise, I did not review the math in this PR, but I'll defer to your expertise. It sounds like a useful addition.

Cheers,
-Kevin

@dmbutyugin
Copy link
Collaborator Author

Thanks for the feedback, Kevin.

One question I have - would it be reasonable to convert from the current /tmp/xxx files to the webhooks api for this data?

This kind of change is orthogonal to my changes in this PR though. I do agree that this could be beneficial, but I think we should consider doing that separately. And there's one more challenge though, specifically for multi-point input shaper testing: it would require some redesign of the GCodes, because unlike bed meshing, which just needs to be done over an area of a bed, you need to be able to run fairly custom sequences of tests to calibrate the input shaper. For example, on my IDEX, I defined the following macro:

[gcode_macro TEST_ALL_RESONANCES]
gcode:
    {% set mid_x = (printer.toolhead.axis_maximum.x + printer.toolhead.axis_minimum.x) %}
    {% set third_x = (printer.toolhead.axis_maximum.x + 2.*printer.toolhead.axis_minimum.x)/3. %}
    {% set mid_y = (printer.toolhead.axis_maximum.y + printer.toolhead.axis_minimum.y) %}
    {% set test_z = 50 %}
    G28
    T0
    G0 X{mid_x} Y{mid_y} Z{test_z} F18000
    TEST_RESONANCES AXIS=x OUTPUT=raw_data POINT={mid_x},{mid_y},{test_z} CHIPS='adxl345 tool0' NAME=t0
    TEST_RESONANCES AXIS=y OUTPUT=raw_data POINT={mid_x},{mid_y},{test_z} CHIPS='adxl345 tool0,adxl345 tool1' NAME=t0_center
    T1
    TEST_RESONANCES AXIS=x OUTPUT=raw_data POINT={mid_x},{mid_y},{test_z} CHIPS='adxl345 tool1' NAME=t1
    TEST_RESONANCES AXIS=y OUTPUT=raw_data POINT={mid_x},{mid_y},{test_z} CHIPS='adxl345 tool0,adxl345 tool1' NAME=t1_center
    PARK_EXTRUDERS_FOR_MIRROR
    ACTIVATE_MIRROR_MODE
    TEST_RESONANCES AXIS=y OUTPUT=raw_data POINT=0,{mid_y},{test_z} CHIPS='adxl345 tool0,adxl345 tool1' NAME=t01_sides
    TEST_RESONANCES AXIS=y OUTPUT=raw_data POINT={third_x},{mid_y},{test_z} CHIPS='adxl345 tool0,adxl345 tool1' NAME=t01_center
    PARK_EXTRUDERS_FOR_MIRROR
    T0

And then calibrate the input shaper by running

~/klipper/scripts/calibrate_shaper.py /tmp/*_x_*_t0.csv
~/klipper/scripts/calibrate_shaper.py /tmp/*_x_*_t1.csv
~/klipper/scripts/calibrate_shaper.py /tmp/*_y_*.csv

for T0 X, T1 X, and Y, so the calibration procedure is fairly elaborate in terms of which points in which order are being tested and also which results are being used for what. One could redefine the API to something like

SHAPER_CALIBRATE_START AXIS=Y
TEST_RESONANCES AXIS=Y POINT=... CHIPS=...
TEST_RESONANCES AXIS=Y POINT=... CHIPS=...
...
TEST_RESONANCES AXIS=Y POINT=... CHIPS=...
SHAPER_CALIBRATE_FINISH NAME=...

and/or we could also export the results for just simple cases for now, but again, I think that goes beyond the scope of what this PR tries to do.

it would avoid having to warn about changing file formats in the future;

Notably, the format of the files produced by the main Klipper facilities does not change, e.g. for TEST_RESONANCES command, and for SHAPER_CALIBRATE with one test point (and for many test points the format is backwards-compatible, though I do not think the files generated by SHAPER_CALIBRATE is used by anyone today, since it produces shaper recommendations right away).

Otherwise, I did not review the math in this PR, but I'll defer to your expertise. It sounds like a useful addition.

Right, so with all that being said, I think it is fine to commit the PR as-is, it does not affect most of the people who do single-point input shaper calibrations, and for those who do multi-point calibration, it should already be beneficial in my opinion.

@KevinOConnor
Copy link
Collaborator

And there's one more challenge though, specifically for multi-point input shaper testing: it would require some redesign of the GCodes

Well, FWIW, you could do the same thing - instead of TEST_RESONANCES ... NAME=t0 and ~/klipper/scripts/calibrate_shaper.py /tmp/*_x_*_t0.csv, it could be something like ``~/klipper/scripts/calibrate_shaper.py --names x_t0. That is, anywhere klipper writes to a file in /tmp` for `calibrate_shaper.py` to find, Klipper could store in memory for `calibrate_shaper.py` to find via the webhooks api.

Notably, the format of the files produced by the main Klipper facilities does not change

Ah, okay - I missed that. It does look like it is new files with new file layouts though. (Potentially more columns in the csv file.)

Right, so with all that being said, I think it is fine to commit the PR as-is

Okay. I'll give a few more days to see if there are other comments. I'll also try to put together some example code of how this could be moved to the webhooks api, but I'm unlikely to get to it in the next few days.

Cheers,
-Kevin

@dmbutyugin
Copy link
Collaborator Author

Well, FWIW, you could do the same thing - instead of TEST_RESONANCES ... NAME=t0 and ~/klipper/scripts/calibrate_shaper.py /tmp/*_x_*_t0.csv, it could be something like ``~/klipper/scripts/calibrate_shaper.py --names __x___t0. That is, anywhere klipper writes to a file in /tmp` for `calibrate_shaper.py` to find, Klipper could store in memory for `calibrate_shaper.py` to find via the webhooks api.

So, as I said, I am not against the idea of exposing the resonances data via webhooks API. I will consider that, but I think it should be discussed and implemented separately. FWIW, at least just exporting the resonance test data via webhooks would be an easy change (so that web frontends could display it already).

Notably, the format of the files produced by the main Klipper facilities does not change

Ah, okay - I missed that. It does look like it is new files with new file layouts though. (Potentially more columns in the csv file.)

Right, as I mentioned, only rather specific very rare cases of file outputs are affected. The majority of the practical uses are not affected. But since it is theoretically possible to produce these files, the reading in the scripts was adapted to handle those cases too.

Okay. I'll give a few more days to see if there are other comments.

Sure, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants