PronunciationAlphabet change breaks backwards compatibility.

[molind]

https://github.com/valhalla/valhalla/blame/181eed995458608ef488db2983d16dca86a20f2f/valhalla/baldr/graphconstants.h#L395C4-L395C4

Since kNone moved from 0 to 5, it's impossible to use data generated by latest valhalla commit using library build prior this commit.

You can close this issue right away. Just wanted to let you know that it's a breaking change.

[nilsnolde]

Oh damn, that sucks.. would be it be of any use to you if we’d revert that change somehow?

Not sure what our policy is with that, it’s already been a few months since that was merged.

[gknisely]

Yeah. This change was released awhile ago (like a year ago) with the initial updates of languages and pronunciations. However, the error was noticed with latest round of languages and pronunciations changes.

You can see that the old version of the code returns a runtime error. The constants were refactored to fix a data problem, but now old code with new data returns a runtime error.

Unfortunately, the old GetTripPronunciationAlphabet was copied from GetTripLegNodeType and it has the same issue. So if we introduce a new NodeType then we will return a std::runtime_error for it as well.

I can't think of a workaround here for this issue.

[molind]

If the new version of Valhalla is able to read old data, it's a minor issue. We'll ensure that the new routing data is available only if the user has the new version of Valhalla, and that's it. Otherwise, we'll have to ship two versions of Valhalla, as we did during the Valhalla 2 to 3 migration.

I'll write about the results.

[kevinkreiser]

for the new node type example, we could allow an option in data building to force "backward compatible mode" and just not use the newer node types. that would be an option to make progress but allow people to opt out of it in a gracefulway.

regarding this particular one, @gknisely can you refresh my memory on why we cant use 0 for kNone? maybe if we rehash the problem we can think of a way to keep it at 0 and undo this break?

[gknisely]

Reading and writing of the data would be broken. We would only read up to the 0. See https://github.com/valhalla/valhalla/blob/master/valhalla/baldr/edgeinfo.h#L63-L73

[kevinkreiser]

@gknisely that doesnt make sense to me, the header is 3 bytes and the part of it that would be 0 would be the last byte, you'd have to read the full last byte before even being able to know that those 3 bits were 0 valued. i have to take a look at the code that reads that again. i know i helped back when we first messed with it but ive totally forgotten it since then 😄