Skip to content

Synchronously validate input operands/activations #572

@inexorabletash

Description

@inexorabletash

When calling a builder method on MLGraphBuilder that takes an MLOperand or MLActivation as an input (i.e. anything but input() and constant()), there is no check that the input is from the same builder.

The only place in the spec that does this is validate MLOperand steps that take an operand and builder.

This algorithm is only called in:

  • concat() with "If validating MLOperand given input and this returns false, then throw a "DataError" DOMException." which is bogus - it should pass input and this.[[builder]].
  • build() asynchronously validates that the output operands are from the same builder, with "If validating MLOperand given operand and this returns false, then reject promise with a TypeError, and abort these steps."

@a-sully raised this in #552 and it was previously mentioned (but without specifics) in #234

Should failure here be synchronous? This is implied by the behavior of concat() and the open ended "If any of the following sub-steps fail, ..." text in the builder methods. I don't see a reason for async failure (except in build itself, since you can't synchronously fail a method that returns a promise) so I'd strongly prefer synchronous failure. (If async, the build() should be augmented to traverse the whole graph.)

Assuming we want synchronous failure we could spec this in a few ways:

  • Add explicit steps like concat() to all builder methods, e.g.: If validating MLOperand given input and this.[[builder]] returns false, then throw a "DataError" DOMException. - repeated for every input (required and optional if given).
  • Define steps for connect which validate and return failure; currently the builder method steps all already include If any of the following sub-steps fail, throw an "OperationError" DOMException.

Either way, concat() needs to be aligned with the other methods (fix to pass this.[[builder]] or remove redundant steps)

This also raises the question of what exception type it should be:

  • "DataError" DOMException, like concat(), or
  • "OperationError" DOMException, like the substeps which include "connect", or
  • TypeError, like build()

Additionally... is there anything other than "same builder" that's not currently validated, but that should be?

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions