Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolution of issue "Non slewing time adjustments #203" V2 #354

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

PaulMartinsen
Copy link
Collaborator

📑 Description

Following extensive discussion in PR #238 and during SDPi Workshop #4 this PR builds on that draft PR with:

  • a new use-case for non-slewing time adjustments,
  • an extension to support versioning MDIB timestamps.

The original draft PR went wonky so this one rebuilds the key changes on a recent master.

☑ Mandatory Tasks

The following aspects have been respected by the pull request assignee and at least one reviewer:

  • Changelog update (necessity checked and entry added or not added respectively)
    • Pull Request Assignee
    • Reviewer

…solve non-slewing adjustments with a new sequence id.

(cherry picked from commit 1956dcd; resolved conflicts: asciidoc/volume1/use-cases/tf1-ch-c-use-case-stad.adoc)
Added new use-case for non-slewing time adjustments
Moved R1522 into new use-case.
@PaulMartinsen PaulMartinsen added Volume 1 Volume 1 contents Volume 3 Volume 3 contents Spec An proposal related to the content or organization of the specification labels Dec 11, 2024
@PaulMartinsen PaulMartinsen added this to the SDPi 2.0 Review milestone Dec 11, 2024
@JavierEspina JavierEspina linked an issue Dec 11, 2024 that may be closed by this pull request
.R1569
[sdpi_requirement#r1569,sdpi_req_level=may]
****
A <<vol1_spec_sdpi_p_actor_somds_participant>> may enter a fault state by, for example, setting the `MdsState/@ActivationState` to `Fail` upon detecting a non-slewing time adjustment that it otherwise cannot recover from.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want this to be even more strict:

While a provider cannot verify coherency of timestamps in the MDIB, for every MDS that is affected, the provider shall set pm:Clock/@ActivationState = Fail.

NOTE: If for technical reasons or risk related measures a provider cannot set a new sequence id after a non-slewing time adjustment ensued, the provider sets the clock into fail mode such that consumers know they must not trust any timestamps that are read from the provider's MDIB.


That includes setting the clock to fail even if epoch versions are used. This allows for the consumers that do not understand epoch versions to act as if the clock would not be reliable anymore. From that it follows that the epoch versioning extension is required to be uniquely identifiable by a consumer as a means to recover from a failed clock.


If a provider does not want to set its clock to Fail, it may produce a new MDIB sequence at any time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not sure I completely follow this one and its interaction with other points. I think it depends on whether we put mustUnderstand="true" on some extension in the clock descriptor, and whether we are trying to deal with participants outside SDPi. This is where I got to:

If we put mustUnderstand="true" on in the clock's descriptor for an versioning extension:

  • consumers have to understand whatever we come up with in SDPi otherwise they must not to process messages.
  • the 3rd option in 1522 provides more nuance on which timestamps are reliable and which are not:
    • sdpi:Epochs/@Version changing signals a abrupt time adjustment,
    • pm:ClockState/@LastSet signals when that occurred (in the current time-reference frame),
    • no timestamps before @LastSet can be relied on even if the consumer understands epoch versioning and the timestamps are versioned because:
      • the reason for the abrupt timestamp can't be determined by the participants, and
      • the timing for the abrupt time adjustment can't be determined by any participant to a resolution any higher than the sync timing.
  • I don't think there's really any downside to setting pm:ClockState/@ActivationState != "On" but in the above context it isn't necessary.
  • We could reserve Fail for clock failures (e.g., hardware fault) and use Stndby for synchronization failures; maybe we don't need to separate these things though.
  • I think the strict part is in 1522 (providers must chose one of the options); 1569 is more about consumers understanding of what the fault state means; I assume providers can go into a fault state whenever they like.
  • Perhaps a pm:AbstractDescriptor/@SafetyClassification="MedA" (display only) might have a relaxed attitude to abrupt time jumps happy to just convey the information while a MedC (clinical function) provider might fault the whole device.
  • I better make 1569 refer to the provider as consumers don't have a activation state to set.
  • Is world time always a vital part of system function contribution? Perhaps a heart-lung machine would be more interested in continuing oxygenation than time-stamping exactly when a time-discontinuity occurred?

If mustUnderstand isn't true, I don't think there is anything about how a consumer should interpret ClockState/@ActivationState != "On". It might surprise them but SDPi conformance testing could validate they do something sensible. Although consumers that are on-board with SDPi should know what do with mustUnderstand=true. Are we worried about non SDPi compliant consumers? This just makes me think we should have mustUnderstand=true.

In conclusion, I'm not sure what to do next here.

@@ -0,0 +1,86 @@
[#vol3_clause_timestamp_versioning]
====== Timestamp versioning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need some sort of a formula to provide guidance as to how a consumer may calculate actual timestamps based on the epoch version and a base/reference timestamp.

So far I did not read the whole text, so the following questions came up:

  • Is the offset based on the previous epoch or based on a reference timestamp?
  • How does this design take into consideration that there may be timestamps affected by non-slewing adjustments in extensions?
  • Do we need the epoch version extension to be mustUnderstand? If yes, we need to show that somewhere in the descriptor as otherwise consumers may bump into a mustUnderstand extension after inspection of the description (perhaps something attached to the decriptor to signify support of epoch versions)

Copy link
Collaborator Author

@PaulMartinsen PaulMartinsen Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I don't think it is possible for a consumer to calculate actual timestamps following an abrupt change in time. Here's an attempt to show why:

Let's say T0 is the current time obtained from a stratum 0 clock, the most accurate timekeeper on the planet, an atomic clock for example. And TP is the time from a provider's clock. Then: $$T_P = T_0 ( 1 + f(T_0)) + \Delta + \eta$$.

Here:

  • $$\Delta$$ is the average difference in time between the reference and provider's clocks,
  • $$\eta$$ is a random number, probably from a normal distribution, that reflects the uncertainty in transferring time from reference clocks through to the provider's clock. I think this would be quite small under the right (or even sensibly typical) conditions; maybe a few milliseconds or less.
  • $$f(T_0)$$ is a scaling function that represents the behaviour of the providers time service client tyring to keep the times in sync. Since non-slewing adjustments (loosely) occur when the difference between reference and provider clocks exceed a bit over four minutes, one possibility for $$f(T_0)$$ might be:
    $$f(T_0) = 5 minutes * T_0$$, although if it was then the provider's clock discipline algorithm should compensate for that (with a slewing adjustment)
    another might be:
    $$f(T_0) = \delta(T_0 - t_{adjust})*2$$, which could be a 2 hour forward adjustment of the reference clock at some point $$t_{adjust}$$; the provider doesn't know exactly when that happens because it is probably checking in with the reference clock every 10 minutes or 2 hours or something.

$$f(T_0)$$ might also be much more complicated. I'd actually expect something more like one of these examples because the clock-discipline algorithm seems to be a closed loop controller so it probably oscillates towards an equilibrium, or maybe (probably) its well tuned and just decays to a small error. Left graph illustrates time from provider plotted against time from precision source; right graph is the difference between them:
image

Note, these don't illustrate the non-slewing adjustment; this is just the slewing behaviour. And they should be considered a novice guess. I can't recall enough of the algorithm to know how it behaves following a non-slewing adjustment; possibly a step change followed by damped oscillations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we can do is be very clear about how the sdpi:Epoch adjustments must be implemented. This is covered inthe schema and 3:8.3.2.10.8.1 Model, but I've added a diagram too since it is critical every implementation does it the same way.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think there should be an extension in the clock descriptor with a must-understand attribute. I think this would prevent consumers that aren't fully up to date with SDPi requirements being surprised by epoch versions as if they don't understand they should cease any further communication with the provider.

I'll add something in the next couple of days to the schema as a starting point. Do you think this would be a place to declare how the provider handles non-slewing adjustments (e.g., new sequence id vs setting activation state), or is this just a guard against surprises?

.R1521
[sdpi_requirement#r1521,sdpi_req_level=should]
****
The <<term_manufacturer>> of a <<vol1_spec_sdpi_p_actor_somds_participant>> should configure its <<acronym_ts_service>> client to give priority to system clock adjustments that are slewing (over stepping adjustments).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using step adjustments / step changes / non-slewing time adjustments interchangeably? If yes: Should be replaced by one term to reduce confusion. Either way, we direly need definitions of terms used in the area of time adjustment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one got missed. I've tried to stick with "non-slewing" and "slewing" adjustments because that's the term mostly commonly used with NTP and it is less confusing than "step" change.

Also, R1521 bothers me a lot. I think its intent can be better expressed (no offense intended to original author :) ) with the drivers for and references to RFC5905 and changed it too:

.R1521
[sdpi_requirement#r1521,sdpi_req_level=should]
****
The <<term_manufacturer>> of a <<vol1_spec_sdpi_p_actor_somds_participant>> should configure
its <<acronym_ts_service>> client to prioritize smooth, monotonic, changes to the system clock. 

.Notes
[%collapsible]
====
NOTE: <<vol1_spec_sdpi_p_actor_somds_participant>>s using, for example, <<ref_rfc_5905, NTP>> 
to syncronize their device clock with the <<acronym_ts_service>> could satisfy this requirement by 
following the cold and warm startup algoriths and clock discipline algorithms with tuning parameters 
described in <<ref_rfc_5905>>.

NOTE: <<vol1_spec_sdpi_p_actor_somds_participant>>s using other synchronization standards
should similarly strongly favour slewing (adjusting clock frequency) over non-slewing (large changes forward 
or backward in time) adjustments, and supress non-slewing adjustments for a period during initialization. 

====
****

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I started a glossary. I didn't know where to put it yet, so for now at the top of the non-slewing use case. Some editorial normalization of terms is still needed to be consistent everywhere — once we have the main points, I guess

Also below for reference:

time-reference frame
A device-specific context for measuring and assigning timestamps to events defined by its rate of passage of time (which may vary over time) and alignment to some external temporal standard (e.g., provided by a <<acronym_ts_service>>). Changes to the time-reference frame, such as non-slewing adjustments, can create distinct epochs with different temporal properties.

epoch
A disctinct period of time characterized by a consistent temporal properties; a single time-reference frame.

timestamp
A point in time obtained from a system clock; while a timestamp is obtained within the context of a time-reference frame, timestamps do not have an intrinsic reference to time-reference frame.

timestamp version
A unique identifier, within the scope of a MDIB sequence, of a time-reference frame epoc.

slewing time adjustment
Adjustments made to a system clock's frequency. Generally so that the time reported by a system clock matches that of a <<acronym_ts_service>> at some point in the future, within the statistical uncertaintity of the synchronization algorithm.

non-slewing time adjustment, abrupt time adjustment
An abrubt change to a system clock's time-reference frame to match the time reported by a system clock with that from a <<acronym_ts_service>>, within the statistical uncertaintity of the synchronization algorithm, as quickly as possible.

smooth time adjustments
A gradual adjustment to the temporal properites of a time-refernece frame, characterised by a continuous and monotonically increasing progression of timestamps without abrupt jumps or disruptions to the passage of time. Generally so that the time reported by a system clock matches that of a <<acronym_ts_service>> at some point in the future, within the statistical uncertaintity of the synchronization algorithm.

clock-discipline algorithm
The algorithm employed by a <<acronym_ts_service>> client to minimize the error between a reference time source. It main include smooth (e.g., slewing) and, in some cases, abrupt (e.g., non-slewing) corrections.

.R1560
[sdpi_requirement#r1560,sdpi_req_level=shall]
****
The <<vol1_spec_sdpi_p_actor_somds_participant>> shall include the determination time of the log entry in both the time-reference frame before, and after, each non-slewing clock adjustment.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the log entry" should be described as the log entries produced by TR1340 of the 10700 (there is no requirement to only have a single log entry showing the time adjustment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarified link:

.R1560
[sdpi_requirement#r1560,sdpi_req_level=shall]
****
The <<vol1_spec_sdpi_p_actor_somds_participant>> shall log each non-slewing adjustment 
of the local clock with an entry that includes the determination time of the log entry in both 
the time-reference frame before, and after, each non-slewing clock adjustment. 

.Notes
[%collapsible]
====

NOTE: This requirement supplements TR1340 in <<ref_ieee_11073_10700_2022>>&mdash;_An 
SDC BASE PARTICIPANT SHOULD log each non-slewing adjustment of the local clock._&mdash; 
requiring specific information in the log to support post incident analysis

====
****


[NOTE]
====
R1520 excludes non-slewing adjustments to the <<acronym_ts_service>> by the RESPONSIBLE ORGANIZATION during normal operation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see R1520 excluding non-slewing adjustments to the TS Service. Could you provide information as to where that is required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is R1520 requires provision of a time-service with a 50 ms accuracy. So either the difference between a primary time source and the time reported by the time-service is never greater than 50 ms or the time-service is the authoritative definition of time for SDC participants. Such a time-source could never require a non-slewing adjustment without violating the requirement so I can't see how it could be the source of one for a participant.

I can't quite see how any problems with the time-service could be resolved. Perhaps by shutting down all SDC devices on the network fixing the clock then restarting everything? But if the time-service is the authoritative definition of time for SDC participants it is always right anyway.

.R1568
[sdpi_requirement#r1568,sdpi_req_level=shall]
****
The <<term_manufacturer>> of a <<vol1_spec_sdpi_p_actor_somds_provider>> that chooses to omit epoch versions from any timestamp shall consider the risks arising from erroneous timestamps.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using epoch versions does not exempt a manufacturer from aussuming erroneous timestamps. So no matter if a provider does or does not uses epoch versions, it needs to assess the risks resulting from erroneous timestamps and install appropriate risk measures

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I thought the risks might be different with and without versioned timestamps. For example, I note that epoch versions may not be required for timestamps on items that update frequently. Perhaps this is obvious though and doesn't merit another requirement.

I don't feel strongly about it; shall I remove this one since it is already covered by RR1162 in 10700:
"The MANUFACTURER of an SDC BASE CONSUMER SHALL consider the RISKs resulting from erroneous timestamps." ?

****
When the <<vol1_spec_sdpi_p_actor_somds_provider>> detects a step adjustment of a system clock, used in making its System Function Contribution (<<acronym_sfc>>), the <<vol1_spec_sdpi_p_actor_somds_provider>> shall either:

* initiate a new MDIB sequence by assigning a new MDIB sequence identifier, or
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list does not cover the option of not setting a new sequence id, but just provding a failed clock.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. My thinking was setting pm:ClockState/@LastSet and updating the sdpi:Epochs/@Version was all that was necessary to communicate the problem with the sync methods allowed. However, this requires the extension. I've added it as #2:

  1. initiate a new MDIB sequence by assigning a new MDIB sequence identifier, or
  2. set pm:ClockState/@ActivationState to StndBy when any timestamp in a <<acronym_mdib>> version was not obtained from the time-reference frame of the active clock in the same version, or
  3. set pm:ClockState/@LastSet to the earliest time that is unambiguously in the current epoch and increment sdpi:Epochs/@Version.

I'm not sure option 2 is worthwhile though because it means leaving the @ActivationState as not "On" until any infrequently updated item (e.g., patient, location context etc) gets a new timestamp, which may be many hours. Still, it could be a useful option for some providers and I have no objection to it.

I'm suggesting StndBy instead of Fail here so that participants could use Fail when their clock itself was faulty. 10207:§B.28 describes StndBy as "The component is ready to be operated but not currently operating", which seems true of the time-stamps the clock component has created so makes sense to me. StndBy could also be used for clocks waiting to synchronize, however they also have ClockState/@RemoteSync and NotRdy to indicate initialization.

asciidoc/volume1/use-cases/tf1-ch-c-use-case-stad.adoc Outdated Show resolved Hide resolved
.R1561
[sdpi_requirement#r1561,sdpi_req_level=may]
****
The <<vol1_spec_sdpi_p_actor_somds_provider>> may indicate a timestamp belongs to a specific epoch using the SDPi epoch extension.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really just a may? Once you start using epoch versions, won't you add epoch version to each timestamp after a non-slewing time adjusment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think "may" makes sense here. For example, a metric that is updated every second doesn't deserve the versioning overhead as its determination time, for example, is almost always in the current time-reference frame.

Almost always because, here, there is a 1 second window in which the MDIB can be requested but the determination time could be from an earlier time-reference frame. This ambiguity is resolved with the LastSet value.

.R1566
[sdpi_requirement#r1566,sdpi_req_level=shall]
****
The <<term_manufacturer>> of a <<vol1_spec_sdpi_p_actor_somds_provider>> that changes the MDIB sequence identifier when it can no longer make smooth adjustments to its time-reference frame shall consider the risks arising from gaps in continuous data.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose smooth adjustments are meant to be slewing adjustments? Is it less, is it more than that?

Copy link
Collaborator Author

@PaulMartinsen PaulMartinsen Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please disregard previous reply; I got it wrong.

Slewing, as I recall, is just how quickly something can change. I remember it from op-amps where slew-rate was how quickly the output voltage would follow a step change to the input voltage. Here, it is one approach (and maybe the best one, and at least one employed in RFC5905) to make smooth adjustments. In other words, we change the tick rate of the device clock. In a perfect world, the clock ticks at 1 s/s. If we're behind we might tick our clock at 1.001s/s; if we're ahead we could tick at 0.999s/s.

But Biceps doesn't require NTP; there's a whole suite of other approaches that could be used with coded values defined in 10101. So "smooth adjustments" is the general term for "slewing adjustments". And I'm trying to use general terms in the requirements but more concrete terms elsewhere to aid understanding (including my own).

@d-gregorczyk
Copy link
Collaborator

General remark: shouldn't we deliver options 1 and 2 and only then continue crafting option 3? I fear it may take some time until we are able to merge option 3 to master.

* Fixed actor in R1560
* Added option to make the activation state of the clock standby while there were timestamps from a different time-reference frame present in the mdib.
* Updated R1521 to express intent and build on established methods.
* added a glossary, temporarily in the use case section
@PaulMartinsen
Copy link
Collaborator Author

General remark: shouldn't we deliver options 1 and 2 and only then continue crafting option 3? I fear it may take some time until we are able to merge option 3 to master.

I don't know. My thoughts FWIW:

  • It seems to me the most useful approach involves an extension because it isn't enough to just set ClockState/@LastSet when the provider isn't using NTP. I'll see if I can think of a way to avoid that.
  • Using only activation state doesn't seem as good when there are long lived things like location and patient contexts. However, since non-slewing adjustments should never happen in practice, maybe it isn't so bad; still making progress?
  • If we need part of an extension we might as well go the whole way.
  • I think we are close to something that, while not perfect, is a big step forward.
  • Perfection is the barrier to progress and change isn't impossible.
  • I'd probably lean towards including something that can be implemented and tested (particularly if there will be a must-understand element) both because the problem isn't going away and folk want to deploy implementations. Maybe we could start with the must-understand as false to avoid breaking changes as a first step?
  • Getting everything finalized by the 14th deadline looks challenging.

But I'll leave the decision to wiser heads and see if I can place some asciidoc ifdefs to give us useful options over the next couple of days.

@ToddCooper
Copy link
Collaborator

SDPi Friday 2024.12.13 -

Still too many open issues. PR pushed to 2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spec An proposal related to the content or organization of the specification Volume 1 Volume 1 contents Volume 3 Volume 3 contents
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Non slewing time adjustments
4 participants