@artofnothingness I pushed a commit here that I think you'll approve of. I changed it use the same maximum + sum as before, and scaled the velocities by the model dt to give positions I added to the critics data (that would generally be useful for some critics to know). This allows this critic to maintain the same order of magnitude from before, representing reversing distance, so we don't need to tune this critic again (or as much).
The immediate on sum is documented in xtensor to really speed things up.
I tested this and happy with the performance. I was pretty happy with it too in your initial commits, but I wanted to try to mimic historical behavior / tuned parameters as much as possible. The CPU change between them is negligible. Now that this critic removes 2 trig functions, I see the spikes less frequently cause issues with the loop rate. Spikes into the ~20s of ms rather than the ~30s of ms.
artofnothingness
commented
1 year ago
@artofnothingness I pushed a commit here that I think you'll approve of. I changed it use the same maximum + sum as before, and scaled the velocities by the
model dtto give positions I added to the critics data (that would generally be useful for some critics to know). This allows this critic to maintain the same order of magnitude from before, representing reversing distance, so we don't need to tune this critic again (or as much).The immediate on sum is documented in xtensor to really speed things up.
I tested this and happy with the performance. I was pretty happy with it too in your initial commits, but I wanted to try to mimic historical behavior / tuned parameters as much as possible. The CPU change between them is negligible. Now that this critic removes 2 trig functions, I see the spikes less frequently cause issues with the loop rate. Spikes into the ~20s of ms rather than the ~30s of ms.
I'm happy to merge this if you are.