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" #238

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JavierEspina
Copy link
Collaborator

πŸ“‘ Description

β˜‘ 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

@JavierEspina JavierEspina linked an issue Oct 27, 2023 that may be closed by this pull request
@JavierEspina JavierEspina self-assigned this Oct 27, 2023
.R1522
[sdpi_requirement#r1522,sdpi_req_level=shall]
****
When the <<vol1_spec_sdpi_p_actor_somds_provider>> detects a stepping adjustment of its system clock, the <<vol1_spec_sdpi_p_actor_somds_provider>> shall initiate a new MDIB sequence by assigning a new MDIB sequence identifier.
Copy link
Collaborator

@PeterKranich PeterKranich Oct 30, 2023

Choose a reason for hiding this comment

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

We see the creation of a new MDIB sequence identifier critical - especially in conjunction with the SDC history service: how can the SDC consumer detect if the new sequence was caused due to a device reboot (data gaps), or a clock adjustment (no data gap)? Does the ClockState still provides the information, when the clock has changed? Is the information in the old or new sequence?
It will also interrupt the continuity of version numbers, although the actual measurement interval was not influenced. This makes gap detection more complex - any suggestions how to do that?

Copy link
Collaborator Author

@JavierEspina JavierEspina Oct 30, 2023

Choose a reason for hiding this comment

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

My basic understanding (which could be wrong) is that in case of sequence change, the consumer should flush all its prior knowledge about the provider. But that does not really answer your good questions, Peter. Maybe @d-gregorczyk has more helpful answers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

SDPi Friday (3.11) Discussion:

  1. Issue will never go away ... never be able to achieve 100% certainty that there will be no time slewing time adjustments
  2. Alternatives? Still under discussion ...
  3. RESOLUTION: (a) Keep R1522 as stated; (b) add note to requirement with Peter's concerns as a "warning"; (c) add a tech advisory requirement indicating the change (to address the last "gap detection" complexity question), including a specification of when the advisory should be sent with respect to the sequence "gap"; (d) [possibly] Add a 2nd advisory alert condition to indicate the reason for a sequence ID change and "gap" in the data

Copy link
Collaborator Author

@JavierEspina JavierEspina Nov 3, 2023

Choose a reason for hiding this comment

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

@PeterKranich, what do you think of the resolution ideas above, from today's SDPi call? If anything is unclear there feel free to reach out to me directly. Oh...and BTW, if (c) and (d) are implemented there may be limited to no need of (b).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SDPi Friday (10 Nov) Discussion:

  • Solutions above do not address the whole issue
  • Alternative idea (direction) is to drop the "new sequence ID" requirement and introduce an extension to the clock state for the provider. Possibly keep an alternative with the "new sequence ID" requirement for those devices that cannot support the extension.
  • Concluded that further discussion is needed, with @d-gregorczyk present. Probably at next week's call about component metrics, time allowing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also talked with my colleges and they agree that creating a new sequence id would have additional implications in addition to the gaps in the contineous data (vitals and waves):

  • Eperiodic data such as an NIBP: what should happen after the new sequence id change? Will the NIBP be invalidated? will the timestamp be corrected the new timestamp? -> this is a really bad idea if the NIBP has already been exported to other systems (EMR) and will then be exported once again with another timestamp.
  • Alert events: what should happen with active alerts? Updating the timestamp and keeping the current occurence id? Or creating a new occurence id? -> this may cause trouble with the alert distribution systems

What my colleges recommend is to treat time like a parameter with a version #. The DeterminationTime would then marked with a time version number. When there is a non-slewing time adjustment, the version would increase and the consumer will get the information of the new time and the difference to the old time.
They pointed out that how these time changes are managed is very application specific and the SDC Provider should just provide all the information to the SDC cousumer which has to decide how to handle the situation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eperiodic data such as an NIBP: what should happen after the new sequence id change? Will the NIBP be invalidated? will the timestamp be corrected the new timestamp? -> this is a really bad idea if the NIBP has already been exported to other systems (EMR) and will then be exported once again with another timestamp.

The sequence id does not change under normal operating conditions. A non-slewing adjustment is an non-recoverable error - so we should treat as one.

Alert events: what should happen with active alerts? Updating the timestamp and keeping the current occurence id? Or creating a new occurence id? -> this may cause trouble with the alert distribution systems

This is actually something that has to be considered in addition by SDPi or 11073-10702 respectively. As far as I can see we do not need to take it into consideration if we were doing a sequence id change, since a sequence id change implicates a breaking change to the system. Moreover, the APKP working group specified calculation of valid alarm limits imposed by particular standards based on the determination time. The calculation is only reasonable when time can be considered correct / strictly increasing.

What my colleges recommend is to treat time like a parameter with a version #. The DeterminationTime would then marked with a time version number. When there is a non-slewing time adjustment, the version would increase and the consumer will get the information of the new time and the difference to the old time.

A time version number does not add any value in our case, it only makes things more complicated. First because consumers have to react upon sequence id changes anyway, and second because we can also use a latched advisory signal as proposed earlier, which signifies a consumer to not rely on any timestamps found in the MDIB.

Bottom line

I think the only reasonable action is to perform a sequence id change. At least it is one valid option. Another one could be to mark all timestamps as tainted by using a latched alert signal. The use and modelling of that alert signal needs to be specified.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Peter and I talked about potential solutions. We did not agree on a particular approach yet, but rather sketched the solution space:

  1. Introduce an extension that allows for a SOMDS provider to signify timestamp validity - if the extension does not exist, the timestamps of a specific entity in the MDIB (aka metrics, alerts, contexts or other extensions that contain timestamps) can be considered up-to-date. We need to discuss if the timestamp validity extension expands to all children of device components or if every state receives its own extension (perhaps preferred as it does not break locality of reference**)
  2. Use a latching alert signal - the signal would apply to the whole MDS or VMDs. No distinction between individual invalid timestamps of metrics and alerts.
  3. Last resort: sequence id change which invalidates everything. This sledgehammer method is currently not preferred, but absolutely valid as sequence id changes can occur at any time - and consumers should be prepared to observe those sequence id changes anyway.

** In Software engineering, all relevant information for understanding a context should be localized at one point in the system and be comprehensible without knowledge of the context

Copy link
Collaborator

Choose a reason for hiding this comment

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

We had discussions at PAT#16 about the options to handle non-slewing time adjustments. We came to the conclusion that an extension may help in organizing timestamps. Each time segment between two non-slewing time adjusments should be versioned (see option 1). Timestamps of one segment can be compared to each other, timestamps of different segments can't be compared.

If a consumer does not want to rely on possibly outdated timestamps, it shall be able identify the latest segment and only use timestamps from that segment. If a consumer is not able to handle the extension at all, an alert signal may help in identifying if the time has been adjusted (see option 2) by a non-slewing event or not, and if there are still timestamps in the MDIB that may be invalid due to this adjustment.

Enhanced solution of option 1: Add deltas to time segments, so that consumers are able to compute the "latest time" based on the sum of deltas to the latest time. Caution: this solution should be thoroughly verified against feasibility and precision.

Copy link

@PaulMartinsen PaulMartinsen Jun 5, 2024

Choose a reason for hiding this comment

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

  1. Introduce an extension that allows for a SOMDS provider to signify timestamp validity - if the extension does not exist, the timestamps of a specific entity in the MDIB (aka metrics, alerts, contexts or other extensions that contain timestamps) can be considered up-to-date. We need to discuss if the timestamp validity extension expands to all children of device components or if every state receives its own extension (perhaps preferred as it does not break locality of reference**)

** In Software engineering, all relevant information for understanding a context should be localized at one point in the system and be comprehensible without knowledge of the context

Is the context here the mdib (that is the mdib has a versioned clock with an e.g. ClockState/@TimeVersion) or every state update (that is, every AbstractState that has a time stamp gets an e.g., @TimeVersion)? It sounds like the later, but the former seems sufficient and in keeping with the rest of mdib updates.

I noticed pm:ClockState has both @DateAndTime and @LastSet. Could @LastSet be useful here if was to track only non-slewing time-adjustments (which includes the first time the clock is set)? Perhaps @OperatingCycles has value to indicate the number of slewing time adjustments that have been taken by the (clock) component as confirmation that clock remains synchronized at the provider end? Updating this count at "each" slewing adjustment would also provide (through @DateAndTime) the slew adjusted time. A bit like the AlertSystemState/@SelfCheckCount.

Finally, is a slewing period requirement going part of SDPi? I haven't been able to find a definitive source whether it is part of the NTP standard, which seems to have many non-slewing adjustments, but some sources indicate it could be system dependent and might be 300s or 600s. 5 - 10 minutes seems like a big clock error for SDC, but I'm not sure I have found the correct information.

@ToddCooper ToddCooper added Volume 1 Volume 1 contents Spec An proposal related to the content or organization of the specification labels Jan 26, 2024
@ToddCooper ToddCooper added this to the SDPi 1.4 review milestone Jan 26, 2024
@ToddCooper
Copy link
Collaborator

Pushed issue resolution to release 1.4 ... after HIMSS'24 Showcase

@ToddCooper
Copy link
Collaborator

SDPi Friday 2024.03.01: During PAT #16 @PeterKranich and @d-gregorczyk discussed and settled on a technical approach. The details have to be updated to this PR. As a result of that + the pushing out of release 1.3 to the end of March, the Milestone was changed back to the original 1.3.

@ToddCooper
Copy link
Collaborator

ToddCooper commented Mar 8, 2024

SDPi Friday 2024.03.08: Technical approach decided during PAT #16 (see comment above). General approach is extending spec to support time stamp version. Details need to be resolved BUT won't be ready by 1.3.

@ToddCooper
Copy link
Collaborator

SDPi Workshop #3 2024.06.04:

Review proposed text PAT #16 discussion
All workshop participants - review by end of workshop.

@ToddCooper
Copy link
Collaborator

SDPi Workshop 3 2024.06.06:

@d-gregorczyk will finalize the language discussed above and do a final review on this PR.

Paul Martinsen will create a new Issue ticket regarding communication of the slewing time (see comment above).

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
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Non slewing time adjustments
5 participants