geez0x1 / CompliantJointToolbox

Compliant Joint Toolbox (CJT) for MATLAB and Simulink
GNU General Public License v3.0
37 stars 14 forks source link

bode_tuy phase correction #10

Closed geezIIT closed 8 years ago

geezIIT commented 8 years ago

bode_tuy corrects phase so that it starts within -90...90 deg, however it does so by looking at the first entry in the phase, the frequency of which is determined by the sampling frequency.

This generally works okay for simulations, but when applied to experimental data the first part of the frequency spectrum might not be excited, so the bode plot for those frequencies will contain garbage.

Fix this by adding an argument to bode_tuy and bode_tuyplot that specifies the frequency range of interest.

geezIIT commented 8 years ago

This should be fixed in fce5bb0abec8787dceea75994289d1237ec2b15e and f9f5043191e78d71f674d16cae1954916d843d32. We still assume the phase at the start of the ROI is within 90 deg. The phase correction has been disabled as it seems to be fixed now that phase is unwrapped properly.

joernmalzahn commented 8 years ago

I would suggest to remove the phase correction in this function. If the phase does not make sens, it is typically the nonsinusoidal data to blame (usually transients, offsets, state convergence ) and not the fft.

Moreover, the assumption that the phase has to start between 0 and -90° is rather restrictive. The famous double integrator, that one wishes to achieve in inverse dynamics control will have a constant phase of -180°. If that is wrapped to -90° this is simply wrong.

The expected behavior when I give a region of interest to a plot function is that it gives me a view on the region i am interested in. I do not expect it to manipulate the graph's values in any way.

I would suggest to remove this. If for any reason, the phase output needs a correction to be displayed properly, it should be first checked, if it can be solved by providing proper innput data (throwing away the transient data points etc.) or otherwise "corrected" for the particular case e.g. call to bode_tuy and then manual plotting.

geez0x1 commented 8 years ago

As stated above, it was already removed, as properly unwrapping solved the issues for which it was created in the first place.