robertknight
Adding attributes to an operator without them breaks backwards compatibility
There is a backwards compatibility hazard with the way operator deserialization works. If an operator initially does not have attributes, and then they are added later, deserializing operators from older model files will fail with a ReadOpError::AttrError error. Instead deserialization should produce an operator with a default configuration that matches how the attribute-less operator deserialized from earlier versions.
AFAIK this issue hasn't come up in practice yet, but I realized it could with the Gelu operator. That currently has no attributes, but if I wanted to add support for the approximate attribute, doing so using the current impl_read_op! macro would lead to a breaking change.
AFAIK this issue hasn't come up in practice yet, but I realized it could with the Gelu operator. That currently has no attributes.
For GELU specifically this was addressed with https://github.com/robertknight/rten/pull/269.
This was solved for the Pad operator by writing a custom ReadOp impl which doesn't error out if attributes are missing. See https://github.com/robertknight/rten/pull/326. I'll do the same for other operators that get attributes added later or have their attribute struct type changed for any reason.