Fix setup issues and improved documentation #39
Conversation
* Fixed broken OSI validator setup for Ubuntu 18.04. * Fixed an issue that would cause .osi messages to be read incorrectly. * Added documentation on how a .txt and .osi trace file has to look like.
| "defusedxml", | ||
| "colorama", | ||
| "tabulate", | ||
| "progress", | ||
| "protobuf==3.9.1", | ||
| "protobuf", |
There was a problem hiding this comment.
Die Begründung (aus unserer internen Kommunikation):
Es sieht so aus, als hätte der osi-validator da ein größeres Problem mit Protobuf3: viele der Checks prüfen, ob ein Feld gesetzt ist, aber in proto3 kann man gar nicht mehr feststellen, ob eine Feld nicht gesetzt wurde, oder den Default-Wert hat.
Beispiel:
TrafficSign.MainSign.Classification.vertically_mirrored.is_set(None) does not comply in SensorView.global_ground_truth.traffic_sign.main_sign.classification
vertically_mirrored ist ein bool. Wenn der Wert bei der Generierung auf false gesetzt wird (der Default), dann wird er in proto3 nicht übertragen.
Damit ist ein größerer Teil der osi-validator checks im Zusammenhang mit proto3 hinfällig.
There was a problem hiding this comment.
Ich erinnere mich es gab vom osi-validator ein Problem bezüglich anderer protobuf Versionen. Deshalb habe ich auch explizit die funktionierende Version verwendet. Wie im Kommentar oben erwähnt würde es sich lohnen gh_actions für osi-validator aufzusetzen. Bevor man den PR merged um die test von dem PR zum laufen zu bringen.
There was a problem hiding this comment.
From my side +1. It is working with our environment.
But I would like to have feedback from @vkresch before merging.
There was a problem hiding this comment.
Es scheint das Problem zu fixen auf meiner lokalen Maschine, aber es scheint auf der pipeline weiterhin zu bestehen (#40). Ich habe die Vermutung, dass es etwas mit der Länge der ints auf der pipeline und unseren lokalen Maschinen zu tun hat. Vielleicht muss man die int Länge hard coden (siehe hier) mit self._int_length=4. Ich werde mir das später noch genauer anschauen.
| "defusedxml", | ||
| "colorama", | ||
| "tabulate", | ||
| "progress", | ||
| "protobuf==3.9.1", | ||
| "protobuf", |
There was a problem hiding this comment.
Ich erinnere mich es gab vom osi-validator ein Problem bezüglich anderer protobuf Versionen. Deshalb habe ich auch explizit die funktionierende Version verwendet. Wie im Kommentar oben erwähnt würde es sich lohnen gh_actions für osi-validator aufzusetzen. Bevor man den PR merged um die test von dem PR zum laufen zu bringen.
| @@ -320,7 +320,7 @@ def get_messages_in_index_range(self, begin, end): | |||
| for rel_index, rel_message_offset in enumerate(rel_message_offsets): | |||
| rel_begin = rel_message_offset + self._int_length | |||
| rel_end = ( | |||
| rel_message_offsets[rel_index + 1] - self._int_length | |||
| rel_message_offsets[rel_index + 1] | |||
There was a problem hiding this comment.
Anscheinend hat protobuf updates bekommen. Ich habs lokal getestet und es funktioniert, wenn man die subtraktion weglässt wie hier. Am besten man setzte auch für den osi-validator github actions auf.
|
@vkresch maybe some things (usage.rst) need to be taken into the new PR and we close this one? |
|
Usage will be clarified in #56 |
Add a description
Check the checklist