NLU returns an empty slot instead of crashing #83
Conversation
…e intent's list of supported slots
There was a problem hiding this comment.
This change looks good on its own. If I'm reading things correctly, though, in order to have full parity across all platforms, we need iOS to ensure that all slots present in the metadata for a given intent are represented in the parser's return value. Slots not tagged by the model get a nil value. Right now, we're only returning slots that the model finds in the input.
|
@space-pope nlu classification output now includes all slots defined by the classified intent |
| // Go over the slots defined by the intent, checking if each one has a value in the model output. | ||
| try intent.slots.reduce(into: [:], { (result, intentSlot) in | ||
| // Check the dictionary of slot name keys with input token values to see if there's a match for this intent slot. | ||
| let taggedSlotValues = tagsToTokens.filter({ $0.key == intentSlot.name }) |
There was a problem hiding this comment.
Isn't tagsToTokens a dictionary? So
if let tokenIndices = tagsToTokens[intentSlot.name] {
// decode and parse tokenIndices
result[intentSlot.name] = parsedSlot
} else {
result[intentSlot.name] = emptySlot
}
And while we're at it, this doesn't really need to be a reduction; it might be easier to read as a dictionary followed by a for-loop to populate.
There was a problem hiding this comment.
ah, that's what i kept trying to get to here, thanks
| // Check the dictionary of slot name keys with input token values to see if there's a match for this intent slot. | ||
| let taggedSlotValues = tagsToTokens.filter({ $0.key == intentSlot.name }) | ||
| if taggedSlotValues.isEmpty { | ||
| // no match, so hyrdate this intent slot with nil |
There was a problem hiding this comment.
| // no match, so hyrdate this intent slot with nil | |
| // no match, so hydrate this intent slot with nil |
The judges would also have accepted "include an empty slot"
There was a problem hiding this comment.
I like the judge here
| // Slots declared by an intent but not tagged by the model are returned to the caller in the output with a null value | ||
| let taggedInputEmpty = ["o", "o"] | ||
| let parsedEmpty = try! parser.parse(tags: taggedInputEmpty, intent: metadata!.intents.filter({ $0.name == "i.i" }).first!, encoder: encoder!, encodedTokens: et) | ||
| XCTAssertNil(parsedEmpty!["iMi"]!.value) |
There was a problem hiding this comment.
The number of exclamation points in the last two lines here is horrifically amusing.
Co-authored-by: Josh <space-pope@users.noreply.github.com>
There was a problem hiding this comment.
Looks good; thanks for all the revisions.
There is one other thing I just noticed, but it's sort of out of the scope of these changes: it doesn't look like Slot includes the original string tagged in the utterance (before we've parsed it into a concrete type). This is pretty much only useful in the case of selsets to give the caller both the normalized and observed value of the slot, but it is something that the other two platforms do.
| // Iterate over the slots defined by the intent, checking if each one has a value in the model output. | ||
| for slot in intent.slots { | ||
| if let tokenIndices = tagsToTokens[slot.name] { | ||
| // decode and parse tokenIndices |
There was a problem hiding this comment.
Heh, this comment was in the suggestion just as a placeholder for "I'm not actually writing the code for this", but I'll take it.
There was a problem hiding this comment.
it was much simpler than my comment
| @@ -14,7 +14,7 @@ import Foundation | |||
| /// - Remark: ex: "up,dog" | |||
| /// - Warning: cannot contain spaces | |||
| /// - SeeAlso: `AppleWakewordRecognizer` | |||
| @objc public var wakewords: String = "spokestack, spoke stack" | |||
| @objc public var wakewords: String = "spokestack, spoke stack, smokestack, smoke stack" | |||
|
Cool, thanks for the feedback, this ended up simpler and better documented than before, with a nice additional feature. |
Does what it says on the tin. For example, "take a selfie" in ios-studio results in
NLUError.metadata("Could not find a slot called item in NLU model metadata.").