[Linuxptp-devel] [PATCH RFC] phc2sys: use PI constants tuned for software time stamping.

[Richard Cochran]

Although the phc2sys program can use three different methods of measuring
clock offset, all of them are based on what essentially boils down to
software time stamping. This patch configures the default constants to
something more appropriate for SW time stamping.

Signed-off-by: Richard Cochran <***@gmail.com>
---
phc2sys.8 | 28 ++++++++++++++++++++++++++--
phc2sys.c | 9 +++------
2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/phc2sys.8 b/phc2sys.8
index 5fcd6f0..56d0adc 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -85,10 +85,34 @@ Specify the slave clock by device (e.g. /dev/ptp1) or interface (e.g. eth1) or
by name. The default is CLOCK_REALTIME (the system clock).
.TP
.BI \-P " kp"
-Specify the proportional constant of the PI controller. The default is 0.7.
+Specify the proportional constant of the PI controller.
+When set to 0.0, the proportional constant will be set by the
+following formula from the current synchronization interval.
+
+kp_scale = 0.1
+.br
+kp_exponent = -0.3
+.br
+kp_norm_max = 0.7
+.br
+kp = min(kp_scale * sync^kp_exponent, kp_norm_max / sync))
+
+The default is 0.0.
.TP
.BI \-I " ki"
-Specify the integral constant of the PI controller. The default is 0.3.
+Specify the integral constant of the PI controller.
+When set to 0.0, the integral constant will be set by the
+following formula from the current synchronization interval.
+
+ki_scale = 0.001
+.br
+ki_exponent = 0.4
+.br
+ki_norm_max = 0.3
+.br
+ki = min(ki_scale * sync^ki_exponent, ki_norm_max / sync)
+
+The default is 0.0.
.TP
.BI \-S " step"
Specify the step threshold of the PI controller. It is the maximum offset that
diff --git a/phc2sys.c b/phc2sys.c
index 2b8af49..bcbff90 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -561,8 +561,8 @@ static void usage(char *progname)
" -c [dev|name] slave clock (CLOCK_REALTIME)\n"
" -d [dev] master PPS device\n"
" -s [dev|name] master clock\n"
- " -P [kp] proportional constant (0.7)\n"
- " -I [ki] integration constant (0.3)\n"
+ " -P [kp] proportional constant (0.0 = automatic)\n"
+ " -I [ki] integration constant (0.0 = automatic)\n"
" -S [step] step threshold (disabled)\n"
" -F [step] step threshold only on start (0.0000001)\n"
" -R [rate] slave clock update rate in HZ (1.0)\n"
@@ -598,9 +598,6 @@ int main(int argc, char *argv[])
.kernel_leap = 1,
};

- configured_pi_kp = KP;
- configured_pi_ki = KI;
-
/* Process the command line arguments. */
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
@@ -794,7 +791,7 @@ int main(int argc, char *argv[])
}
}

- dst_clock.servo = servo_create(CLOCK_SERVO_PI, -ppb, max_ppb, 0);
+ dst_clock.servo = servo_create(CLOCK_SERVO_PI, -ppb, max_ppb, 1);

if (pps_fd >= 0) {
servo_sync_interval(dst_clock.servo, 1.0);

[Miroslav Lichvar]

Post by Richard Cochran
Although the phc2sys program can use three different methods of measuring
clock offset, all of them are based on what essentially boils down to
software time stamping. This patch configures the default constants to
something more appropriate for SW time stamping.

I agree the default HW constants may be a bit too aggressive for
phc2sys, but I think switching to the SW defaults would worsen the
time accuracy in the "average" case.

The jitter I observe with phc2sys (PHC->system clock) is usually in
tens or at most hundreds of nanoseconds. That's at two or three orders
of magnitude better than the PTP jitter with SW time stamping
(microseconds or tens of microseconds). What jitter do others usually
see?

I ran phc2sys in the simulator to see how much would the switch to the
SW constants change the time and frequency accuracy. The following
table contains the differences in RMS time error and RMS freq error
(in dB) between the default SW and HW constants for 10ppb/s wander and
various update intervals and jitters.

u\jit| 1e-8 | 1e-7 | 1e-6 | 1e-5 |
2^-7 | 30 -8 | 15 -17 | -2 -17 | -10 -17 |
2^-6 | 30 -6 | 15 -17 | -3 -17 | -10 -17 |
2^-5 | 32 -4 | 15 -16 | -4 -17 | -10 -17 |
2^-4 | 32 -2 | 17 -16 | -2 -18 | -10 -18 |
2^-3 | 32 0 | 17 -16 | -2 -18 | -10 -18 |
2^-2 | 32 2 | 18 -15 | -1 -19 | -10 -19 |
2^-1 | 32 2 | 18 -14 | -2 -20 | -11 -20 |
2^+0 | 32 2 | 17 -14 | -2 -21 | -12 -22 |
2^+1 | 27 3 | 19 -7 | 1 -17 | -9 -17 |
2^+2 | 21 3 | 19 -2 | 4 -12 | -7 -13 |
2^+3 | 15 1 | 14 0 | 6 -7 | -4 -8 |
2^+4 | 8 -0 | 8 -0 | 6 -2 | -2 -3 |
2^+5 | 4 -0 | 4 -0 | 3 -1 | -1 -2 |
2^+6 | 0 -0 | -0 0 | -0 -0 | 0 0 |
2^+7 | 0 0 | -0 0 | -0 0 | -0 -0 |

(http://mlichvar.fedorapeople.org/clknetsim/ptp/pi_diff/run1d)

While the frequency error is in most cases better with the SW
constants, the time error is significantly worse with 10ns and 100ns
jitter. If you think it's necessary to change the defaults, could we
go with something between the SW and HW constants instead?

Post by Richard Cochran
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -85,10 +85,34 @@ Specify the slave clock by device (e.g. /dev/ptp1) or interface (e.g. eth1) or
by name. The default is CLOCK_REALTIME (the system clock).
.TP
.BI \-P " kp"
-Specify the proportional constant of the PI controller. The default is 0.7.
+Specify the proportional constant of the PI controller.
+When set to 0.0, the proportional constant will be set by the
+following formula from the current synchronization interval.
+kp = min(kp_scale * sync^kp_exponent, kp_norm_max / sync))

In phc2sys it's the update rate what is configurable, maybe it would
be more clear to write the formula with "kp_norm_max * rate" instead
of "kp_norm_max / sync".

[Richard Cochran]

Post by Miroslav Lichvar
While the frequency error is in most cases better with the SW
constants, the time error is significantly worse with 10ns and 100ns
jitter. If you think it's necessary to change the defaults, could we
go with something between the SW and HW constants instead?

Would you care to make a suggestion? I think we can let phc2sys
program its own values for configured_pi_k[pi]_scale and have the -P
and -I command line options override the defaults.

Please also take a look at Rich Schmidt's results in the thread on
linunxptp-users.

Thanks,
Richard

[Miroslav Lichvar]

Post by Richard Cochran

Post by Miroslav Lichvar
While the frequency error is in most cases better with the SW
constants, the time error is significantly worse with 10ns and 100ns
jitter. If you think it's necessary to change the defaults, could we
go with something between the SW and HW constants instead?

Would you care to make a suggestion? I think we can let phc2sys
program its own values for configured_pi_k[pi]_scale and have the -P
and -I command line options override the defaults.

Yes, let it use the automatic scaling instead of setting the values
directly. I'm wondering why it wasn't done like that before.

As for phc2sys having its own default values, I'm not sure. I tried to
tweak the HW constants a bit, but wasn't very happy with the results.
I think the default HW constants may not be that bad after all.

Post by Richard Cochran
Please also take a look at Rich Schmidt's results in the thread on
linunxptp-users.

The original problem seems to be that the PI constants were set
directly in phc2sys and were not scaled to the 32s update interval.
They were only clamped by the servo's stability limit to 1.0/32 and
2.0/32.

If we compare how are the constants set with phc2sys -R 0.03125 in the
current version, with the patch using default SW constants and the
patch using default HW constants, we can see the HW and SW constants
at the 32s interval are very similar.

forced: PI servo: sync interval 32.000 kp 0.031 ki 0.062500
SW scaling: PI servo: sync interval 32.000 kp 0.022 ki 0.004000
HW scaling: PI servo: sync interval 32.000 kp 0.022 ki 0.009375

If you look at row 2^+5 in the table from my previous mail, you'll see
the differences in time error and frequency error are negligible. I'd
propose to change the patch to simply use the HW constants. A nice
bonus is that the default PI constants will be identical at the
default update rate.

What do you think?

[Keller, Jacob E]

-----Original Message-----
Sent: Saturday, November 09, 2013 8:14 AM
Subject: [Linuxptp-devel] [PATCH RFC] phc2sys: use PI constants tuned
for software time stamping.
Although the phc2sys program can use three different methods of measuring
clock offset, all of them are based on what essentially boils down to
software time stamping. This patch configures the default constants to
something more appropriate for SW time stamping.
---
phc2sys.8 | 28 ++++++++++++++++++++++++++--
phc2sys.c | 9 +++------
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/phc2sys.8 b/phc2sys.8
index 5fcd6f0..56d0adc 100644
--- a/phc2sys.8
+++ b/phc2sys.8
@@ -85,10 +85,34 @@ Specify the slave clock by device (e.g. /dev/ptp1)
or interface (e.g. eth1) or
by name. The default is CLOCK_REALTIME (the system clock).
.TP
.BI \-P " kp"
-Specify the proportional constant of the PI controller. The default is 0.7.
+Specify the proportional constant of the PI controller.
+When set to 0.0, the proportional constant will be set by the
+following formula from the current synchronization interval.
+
+kp_scale = 0.1
+.br
+kp_exponent = -0.3
+.br
+kp_norm_max = 0.7
+.br
+kp = min(kp_scale * sync^kp_exponent, kp_norm_max / sync))
+
+The default is 0.0.
.TP
.BI \-I " ki"
-Specify the integral constant of the PI controller. The default is 0.3.
+Specify the integral constant of the PI controller.
+When set to 0.0, the integral constant will be set by the
+following formula from the current synchronization interval.
+
+ki_scale = 0.001
+.br
+ki_exponent = 0.4
+.br
+ki_norm_max = 0.3
+.br
+ki = min(ki_scale * sync^ki_exponent, ki_norm_max / sync)
+
+The default is 0.0.
.TP
.BI \-S " step"
Specify the step threshold of the PI controller. It is the maximum offset that
diff --git a/phc2sys.c b/phc2sys.c
index 2b8af49..bcbff90 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -561,8 +561,8 @@ static void usage(char *progname)
" -c [dev|name] slave clock (CLOCK_REALTIME)\n"
" -d [dev] master PPS device\n"
" -s [dev|name] master clock\n"
- " -P [kp] proportional constant (0.7)\n"
- " -I [ki] integration constant (0.3)\n"
+ " -P [kp] proportional constant (0.0 = automatic)\n"
+ " -I [ki] integration constant (0.0 = automatic)\n"
" -S [step] step threshold (disabled)\n"
" -F [step] step threshold only on start (0.0000001)\n"
" -R [rate] slave clock update rate in HZ (1.0)\n"
@@ -598,9 +598,6 @@ int main(int argc, char *argv[])
.kernel_leap = 1,
};
- configured_pi_kp = KP;
- configured_pi_ki = KI;
-
/* Process the command line arguments. */
progname = strrchr(argv[0], '/');
progname = progname ? 1+progname : argv[0];
@@ -794,7 +791,7 @@ int main(int argc, char *argv[])
}
}
- dst_clock.servo = servo_create(CLOCK_SERVO_PI, -ppb,
max_ppb, 0);
+ dst_clock.servo = servo_create(CLOCK_SERVO_PI, -ppb,
max_ppb, 1);
if (pps_fd >= 0) {
servo_sync_interval(dst_clock.servo, 1.0);
--
1.7.10.4

Thanks Richard, this looks good to me :)

Regards,
Jake