DSDL compatibility analysis #9
Comments
|
@pavel-kirienko i think the most important question is the code compatibility. You seem to opting out of it completely when writing the specification. We should discuss the value of being able to do invasive changes within one major version vs stability in code when using the same major version. Even though I've described the most conservative approach there exists middle grounds. For instance not guaranteeing that fields will be of the same type, but all valid const assignments before the update are also valid after the update. The argument for doing invasive changes within one major version you presented the last time was #5 (comment) i think this problem is not very big as it can be fixed with bit masks and would probably have been fixed in the 0.X phase, and therefore not a good reason to allow breaking code compatibility. Do you have examples with more serious problems? The reason we need (even if it turns out to be a more relaxed sort) code compatibility is that the code generated with the dsdl compiler will also need to adher to semantic versioning for the dsdl types to be truly useful in dependency systems using semver (like cargo). I think a stricter form of code compatibility will produce code that is nicer to use, so this will be a compromise. |
I have added a
I think this is a bad idea. The specification should only care about bit compatibility and semantic compatibility; adding more guarantees undermines the extensibility of the standard. Yes, that implies that when an application migrates from one minor datatype version to another it may have to update the code that uses the datatype, but that is acceptable and doesn't even go against the principles of semantic versioning. What we need to be focused on is whether different nodes can successfully communicate and cooperate using data type definitions with different minor version numbers. The following example provided in the specification illustrates that well: First definition: # demo.Pair
float16 first
float16 second# demo.PairVector
demo.Pair[3] vectordemo.PairVector pair_vectorSecond definition: float16 first_0 # pair_vector.vector[0].first
float16 second_0 # pair_vector.vector[0].second
float16 first_1 # pair_vector.vector[1].first
float16 second_1 # pair_vector.vector[1].second
float16 first_2 # pair_vector.vector[2].first
float16 second_2 # pair_vector.vector[2].secondThe code compatibility guarantee would partially defeat the advantages of the new version management logic. On the subject of binary compatibility: I've been silent lately because of this short statement you made in a different thread:
I thought that I'm just going to throw in a few words to clarify things. However, what actually happened is that I opened a huge can of worms with a whole universe in it. I was sketching some bit patterns in my notepad as I received a notification about this issue; now let me go back to it. |
Let's address this concern before moving on to the bigger concerns.
That's nice. I was playing around with a similar idea myself. What i was thinking about was adding
Even if we initially only supported |
|
I like the flexibility argument, but I have two questions:
If the two options mentioned above are removed, we're back to just I suggest keeping |
|
I split the deprecation issue out to: #10 for making the issues easier to follow from the outisde |
Isn't the problem here essentially that a "valid encoding" and a "valid decoding" is two seperate things, and therefore defining a single "valid encoded representation" term is impossible (or at least needs to be the most liberal of these). I think if we can change the definition to: Bit compatibilityA structure definition
Or a bit more verbose. Let's define Let's define An encoding is a function from On the other hand, a decoding is a function from Let's define For bit compatibility between the definitions that constructs
|
So my approach was different - I was attempting to avoid defining the decoding function completely, expressing the relations in terms of the encoding function only: A is compatible with B if the set of all possible encoded representations of A is a superset of all possible encoded representations of B. However, void fields were giving me trouble, because they are by definition to be encoded as zero, which breaks the set relationship if at least one void bit is to be replaced with a non-void bit. I also tried defining an extended set of all possible encoded representations by allowing the void fields to take arbitrary values, but that created some ugly corner cases with dynamic fields (specifically, array length prefixes and union discriminators could no longer consume void bits because that broke the set relationship as well, making the types incompatible when they should be). Your very last equation makes perfect sense. Let me try continuing with your approach. |
|
Wait no, loss of information may occur at |
Since the definition is concerned with First observe that If we first define c as The consequence is that it's not backwards compatible to remove void fields (or generally remove information) but it is backwards compatible to populate void fields (or generally add information), i think this was what we wanted? Ìf you instead would like the relaxed definition where only the first condition are present. You're essentially asking for serialization compatibility as defined in OpenCyphal-Garage/uavcan.github.io#40 |
That seems overly strict. One can imagine a use case where a new void field would need to be introduced, e.g. if a non-void field became deprecated or split into several smaller fields. I suggest staying generic; perhaps we should move this conversation back to #11 where I have proposed a very minimalistic formulation. |
|
I think we need to talk about the concrete things we expect to have from compatibility data types and their concrete value: ProblemsPopulating void fieldsI think populating void fields is the prime example of what we're 100% sure that we want and doesn't cost us anything. The constraint is that newly populated fields must always be considered as an "optional" with a None variant of the default void serialization value. The following example is overly pedantic about this. ExampleRemoving informationAny form of removing information must be forbidden. If this is not true we must expect that every important piece for information can be removed in the future. This is a destructive way to have to think when working with uavcan. And removing fields should always be semantically incompatible. Example fieldsExamples constsUnrolling arrays/structs or merging fieldsUnrolling or enrolling structs and arrays and merging of fields can make the data types more convenient for the user, but it does not add or remove any information. The problem with this is that it also breaks code compatibility which is inconvenient for users. Instead of doing this on released data types we should experiment enough in the pre-releases that we "get it right". If we fail, we should still not do this for released datatypes as the code breakage is much more severe and annoying than the benefit of being able to make these changes. Example unrolling/enrolling (stolen from Pavel)# demo.Pair
float16 first
float16 second# demo.PairVector
demo.Pair[3] vectordemo.PairVector pair_vectorSecond definition: float16 first_0 # pair_vector.vector[0].first
float16 second_0 # pair_vector.vector[0].second
float16 first_1 # pair_vector.vector[1].first
float16 second_1 # pair_vector.vector[1].second
float16 first_2 # pair_vector.vector[2].first
float16 second_2 # pair_vector.vector[2].secondExample merging fields (stolen from Pavel)# Alert flags are allocated at the bottom (from bit 0 upwards)
uint64 DC_UNDERVOLTAGE = 1 << 0
uint64 DC_OVERVOLTAGE = 1 << 1
uint64 DC_UNDERCURRENT = 1 << 2
uint64 DC_OVERCURRENT = 1 << 3
uint64 CPU_COLD = 1 << 4
uint64 CPU_OVERHEATING = 1 << 5
uint64 VSI_COLD = 1 << 6
uint64 VSI_OVERHEATING = 1 << 7
uint64 MOTOR_COLD = 1 << 8
uint64 MOTOR_OVERHEATING = 1 << 9
uint64 HARDWARE_LVPS_MALFUNCTION = 1 << 10
uint64 HARDWARE_FAULT = 1 << 11
uint64 HARDWARE_OVERLOAD = 1 << 12
uint64 PHASE_CURRENT_MEASUREMENT_MALFUNCTION = 1 << 13
# Non-error flags are allocated at the top (from bit 63 downwards)
uint64 UAVCAN_NODE_UP = 1 << 56
uint64 CAN_DATA_LINK_UP = 1 << 57
uint64 USB_CONNECTED = 1 << 58
uint64 USB_POWER_SUPPLIED = 1 << 59
uint64 RCPWM_SIGNAL_DETECTED = 1 << 60
uint64 PHASE_CURRENT_AGC_HIGH_GAIN_SELECTED = 1 << 61
uint64 VSI_MODULATING = 1 << 62
uint64 VSI_ENABLED = 1 << 63
# Status flags, see the constants above
uint64 flags# Alert flags are allocated at the bottom (from bit 0 upwards)
uint32 ALERT_DC_UNDERVOLTAGE = 1 << 0
uint32 ALERT_DC_OVERVOLTAGE = 1 << 1
uint32 ALERT_DC_UNDERCURRENT = 1 << 2
uint32 ALERT_DC_OVERCURRENT = 1 << 3
uint32 ALERT_CPU_COLD = 1 << 4
uint32 ALERT_CPU_OVERHEATING = 1 << 5
uint32 ALERT_VSI_COLD = 1 << 6
uint32 ALERT_VSI_OVERHEATING = 1 << 7
uint32 ALERT_MOTOR_COLD = 1 << 8
uint32 ALERT_MOTOR_OVERHEATING = 1 << 9
uint32 ALERT_HARDWARE_LVPS_MALFUNCTION = 1 << 10
uint32 ALERT_HARDWARE_FAULT = 1 << 11
uint32 ALERT_HARDWARE_OVERLOAD = 1 << 12
uint32 ALERT_PHASE_CURRENT_MEASUREMENT_MALFUNCTION = 1 << 13
uint32 alert_flags
uint32 STATUS_UAVCAN_NODE_UP = 1 << 24
uint32 STATUS_CAN_DATA_LINK_UP = 1 << 25
uint32 STATUS_USB_CONNECTED = 1 << 26
uint32 STATUS_USB_POWER_SUPPLIED = 1 << 27
uint32 STATUS_RCPWM_SIGNAL_DETECTED = 1 << 28
uint32 STATUS_PHASE_CURRENT_AGC_HIGH_GAIN_SELECTED = 1 << 29
uint32 STATUS_VSI_MODULATING = 1 << 30
uint32 STATUS_VSI_ENABLED = 1 << 31
uint32 status_flagsConclusionIf we were to break "code compatibility" it should be for the ability to add new features. I do not see enough value in mixing around with existing datatypes to justify doing so. |
What is the point of deprecating non-void fields? We cannot repopulate it in a sound way. If every field can be removed you do not have much guarantee as a user of a data type either. We should not be able to remove features (anything) and still call something compatible. |
|
I think it is important to ensure that our reasoning here is guided by the top-level design objectives and not by one's vision on how to implement the best diagnostic tools. ;) It is crucial to ensure that we have all possible tools at our disposal when it comes to the ability to advance data type definitions while not breaking backward compatiblity with existing nodes, because updating fielded nodes is hard and expensive. The balance between ease of coding and ease of maintenance should be skewed towards the latter, which is why I argue that making code compatiblity mandatory is a step in a wrong direction.
This issue is supposed to be managed by the semantic compatiblity requirement. It should be possible to remove constants that ended up being unnecessary, or fields that turned out to be redundant. My example that you've provided in your earlier post here can be easily modified such that the existing wide integer field is split into two smaller fields with a void field in the middle - this is an example of a meaningful transition from a non-void field to a void field. |
Please arrest me wherever I make this mistake.
I agree in the essence but would phrase this differently. Our main objective should be to never risk breaking fielded nodes that are using only stabilized definitions. If I were going to put these definitions in a "fielded node" I would want a strong guarantee of stability. More than "some guy thought this would be fine for most applications". Being able to upgrade definitions is super-duper-mega-nice, but secondary to not breaking stuff. Before the ongoing protocol update there were not even a concept of updating without breaking compatibility, so I don't agree that it's essential to be in the "extreme liberal" corner when it comes to changes we allow to do to definitions. Our main line of defense should be to only stabilize definitions after they are audited and tested thoroughly. We should not expect compatible updating to be a super flexible "save us from anything" kind of tool. But instead allow stabilizing definitions a bit earlier since we can accommodate for adding information at a later point.
In my point of view this is actually about "ease of coding" vs "ease of coding". Conceptually you will not be able to remove any stabilized features without risking to break how fielded nodes work. This means that not having code compatibility will not allow you to do more invasive changes to functionality. The changes you could potentially do are already (or at least should be) limited by "other types of compatibility". The only changes you will be able to do is changes to representation. Unrolling static arrays and such. For the record: If ditching code compatibility can give us any concrete tools to not break fielded nodes I'm all for it. I just currently think that ditching code compatibility give us very little in return. And there are more powerful and robust ways to achieve what we actually wants.
The point about stabilization is committing to something. Once something is stabilized we should absolutely be stuck with this commitment. You can never know if something is "unnecessary" for everyone in the whole wide world. Meaning, once something is stabilized it might be in use, and to avoid breaking fielded nodes we must not remove it. To take your concrete example. If the void field you mentioned were to be reused, old nodes might receive a different status code than what was sent due to the "new field" interfering with the long version of the old field. Meaning that this concrete change you're describing, must be forbidden. If we want to make this possible, it's our responsibility to provide an initial definition that will not be possible to break fielded nodes by changing. Defining data types in a smart way.I think we already have the required tools to make your example possible. We just have to think things through from the start. Since we're able to populate void fields now (contrary to when the definition you're using as an example was made) the following definition should be used if it's important that the unused space can be re-purposed to anything. Having your cake and eating it tooThe DSDL semantics are are not optimized towards writing "upgradable definitions", this makes sense as upgrading definitions was not a part of the original specification. Even though your last example is impossible to make fully backwards compatible with the current semantics, some small changes might make it achievable. If we want to retain the possibility of making as many changes as possible post stabilization we will need to look at other parts of the protocol to make this happen. Note: The following features I'm about to present is not suggested for inclusion. It's only meant as an example of how features that encapsulate information can give more flexible ways of upgrading without breaking any type of compatibility. I created these examples in 3mins, and there probably exists much better types that would allow the same thing with more flexibility. TypedefsLet's introduce typesdefs. We can make it a rule that typedefs are considered private and can be changed with minor versions. The definition may then look like the following: type Alert = uint14
type Status = uint8
Alert alert
void42 # This field can be used to grow status upwards or error downwards, or for something else.
Status statusAd-hoc structsWhat if we allowed ad-hoc structs that we don't expose/assume to much about (this is actually quite similar to typedef in expressive power). We will not remove any fields, but we're free to add fields/consts as long as the top level structure remains compatible. struct alert {
uint1 alert_dc_undervoltage
uint1 alert_dc_overvoltage
uint1 alert_dc_undercurrent
uint1 alter_dc_overcurrent
uint1 alert_cpu_cold
uint1 alert_cpu_overheating
uint1 alert_vsi_cold
uint1 alert_vsi_overheating
uint1 alert_motor_cold
uint1 alert_motor_overheating
uint1 alert_hardware_lvps_malfunction
uint1 alert_hardware_fault
uint1 alert_hardware_overload
uint1 alert_phase_current_measurement_malfunction
}
void42 # This field can be used to grow status upwards or error downwards, or for something else.
struct status {
uint1 status_uavcan_node_up
uint1 status_can_data_link_up
uint1 status_usb_connected
uint1 status_usb_power_supplied
uint1 status_rcpwm_signal_detected
uint1 status_phase_current_agc_high_gain_selected
uint1 status_vsi_modulating
uint1 status_vsi_enabled
}Dynamic range uintsThis feature is not very elegant or smart. It's also way to obscure to be considered for addition. I'm aware of all this. Remember: It is only included as an example Let's introduce the uint1[<=(Y-X)]
uint1[X]Our original definition may then might look like this: dyn_uint(13-28) NO_ERROR = 0 # will be serialized as 4 + 13 bits as small enough to fit inside 13 bits
dyn_uint(13-28) alert # serializes to maximum 32 bits
dyn_void(0-27) # can be replaced with other type of bit fields can be added here.
dyn_uint(13-28) status # serializes to maximum 32 bitsThis will allow both for checking if Breaking the law of excluded thirdThere exists a possibility of doing the following:
We would then get some experience with stabilization before deciding if we want the liberal or conservative approach to compatibility. The drawback is that if we decide to introduce more advanced DSDL concepts that encapsulate information we would not be able to use them for the types we had already stabilized. |
Will do. I apologize for the assumption; the world seems different from where I sit. First, let me introduce two simple empirical counter-arguments to the following two propositions:
Consider the field
In the ticket #13 there is a suggestion to use compound fixed-point arithmetic scalars consisting of two adjacent integer types until there is proper support at the language level: uint16 integer_bits
uint8 fractional_bitsWhich one could foresee replaced with the below alternative once the required support is in place: ufix16_8 numberNow, let's put that aside. Continuing the conversation at the object level will not move us any further. There is a spectrum of possible approaches to the problem of data type maintenance, ranging from the most conservative, where data types are frozen forever once released, to the most liberal, where things are never set in stone and always changing. Let me recap the options:
The Conservative Extreme shields us from mistakes introduced after a major version is released, assuming that we do the job of stabilizing it once, and we do it right. The Liberal Extreme tolerates some mistakes introduced while stabilizing major versions by allowing us to make drastic changes to the following newer minor versions (although they still have to be semantically compatible and bit compatible). The catch here is that while bit-compatibility has a strict formal definition and therefore can be ensured by a computer, semantic compatibility does not, which leaves the question "are these changes safe to introduce" dependent on human judgment. The catch described last is seemingly the main perceived deal-breaker for the case of the Liberal Extreme:
The hypothetical guy thinking about what is good and what is bad for most applications is the driving force behind every complex technical standard out there. It works for USB 3.0, IPv6, and ISO C++; it works for UAVCAN, and there doesn't seem to be a convincing reason it won't work at a smaller scope of individual data types. Disregarding even that: one can see that the freedoms of data type evolution provided by the Conservative Extreme are a subset of those offered by the Liberal Extreme, meaning that one could establish a workflow permitting only the Conservative option, leaving the Liberal option as a very last resort. I would like the specification to be very permissive here. I would like it to put as few constraints as possible, letting data type designers work around the dangers of the Liberal approach on a per-project base, as necessary. |
|
I am closing this as the discussion seems to have migrated to the forum completely: |
I've open this issue to discuss how changes to data types should be handled. This issue is a follow up on OpenCyphal/public_regulated_data_types#35, #5 and #6 but tries to isolate the compatibility discussion without blending it together with other review related items.
I will attempt to keep the text under the line updated to reflect the consensus of the discussion. The initial text is based on previous discussion and (where there are little discussion) my first instincts. Please criticize so we can improve upon it.
Compatibility
Compatibility is a relationship that message definition
Acan have with message definitionB. IfAis compatible withBandBis compatible withAwe say they are mutually compatible. The different forms of compatibility is defined below.Bit compatibility
A structure definition
Ais bit-compatible with a structure definitionBif any valid encoded representation ofBis also a valid encoded representation ofA.Semantic compatibility
A data structure definition
Ais semantically compatible with a data structure definitionBif an application that correctly usesAexhibits a functionally equivalent behavior to an application that correctly usesB, andAis bit-compatible withB.Code compatibility
We say that
Ais code compatible withBif valid use of code generated fromBalso is valid use of code generated fromAwith the same spec-adhering compiler.For a compiler to be spec-adhering it must generate compatible code if:
Bmust also exist inA. They must also have the same type.Bmust also exist inA. They must also have the same type.ID compatible
We say that
Ais ID compatible withBIfBhas a default ID, andAhas exact same default ID orBdoes not have a default ID.Compatibility guarantees
0, no compatibility guarantees are given.0and identical path the version with the highest version number will have the following compatibilities with the other version.Mutability guarantees
All definitions are immutable. This means that after a definition is published a new definition with the same namespace and name must have a higher version number. There are a few exceptions:
Open questions
The text was updated successfully, but these errors were encountered: