Add options to support doing luis and qna cross-train seperately (#1161)

* add flag for controling cross train

* change naming

* add more tests and docs

* rename to trainingOpt

* minor fixes

* add options for luRecognizerID

* fix tests

* retrriger

* fix lint problem

* fix flags naming and add options

* remove lu recognizer id

* fix format

* remove no need doc

* remove a console.log statement

Co-authored-by: Fei Chen <[email protected]>

Author: Shuai Wang

packages/cli/README.md

@@ -839,6 +839,10 @@ OPTIONS
   --intentName=intentName  [default: _Interruption] Interruption intent name
 
   --log                    Writes out log messages to console
+
+  --inner-dialog            Only performs the inner dialog cross train, defalt is true, to set it as false, use --no-inner-dialog
+
+  --intra-dialog            Only performs the intra dialog cross train, defalt is true, to set it as false, use --no-intra-dialog
 ```
 
 _See code: [@microsoft/bf-luis-cli](https://github.com/microsoft/botframework-cli/tree/master/packages/luis/src/commands/luis/cross-train.ts)_

packages/lu/src/parser/cross-train/cross-train.js

@@ -15,9 +15,10 @@ module.exports = {
    * @param {string} intentName interruption intent name. Default value is _Interruption.
    * @param {string} config path to config file of mapping rules.
    * @param {boolean} verbose verbose to indicate whether log warnings and errors or not when parsing cross-train files.
+   * @param {inner: boolean, intra: boolean} trainingOpt trainingOpt indicates whether you want to control do the inner or intra dialog training seperately
    * @returns {luResult: any, qnaResult: any} trainedResult of luResult and qnaResult or undefined if no results.
    */
-  train: async function (input, intentName, config, verbose) {
+  train: async function (input, intentName, config, verbose, trainingOpt) {
     // Get all related file content.
     const luContents = await file.getFilesContent(input, fileExtEnum.LUFile)
     const qnaContents = await file.getFilesContent(input, fileExtEnum.QnAFile)
@@ -53,7 +54,8 @@ module.exports = {
       configId: configContent.id,
       intentName,
       verbose,
-      importResolver
+      importResolver,
+      trainingOpt
     })
 
     return trainedResult

packages/lu/src/parser/cross-train/crossTrainer.js

@@ -27,7 +27,7 @@ module.exports = {
    * @param {any[]} luContents the lu content array whose element includes path and content
    * @param {any[]} qnaContents the qna content array whose element includes path and content
    * @param {any} configObject cross train config json object
-   * @param {any} options some optional parameters including configId, intentName, verbose, importResolver
+   * @param {any} options some optional parameters including configId, intentName, verbose, importResolver, trainingOpt
    * @returns {Map<string, LUResource>} map of file id and luResource
    * @throws {exception} throws errors
    */
@@ -38,18 +38,19 @@ module.exports = {
       const crossTrainConfig = fileHelper.getConfigObject(
         configObject,
         options.intentName || '_Interruption',
-        options.verbose || true)
+        options.verbose || true,
+        options.trainingOpt || {inner: true, intra: true})
 
       let {luObjectArray, qnaObjectArray} = pretreatment(luContents, qnaContents)
-      const {rootIds, triggerRules, intentName, verbose} = crossTrainConfig
+      const {rootIds, triggerRules, intentName, verbose, trainingOpt} = crossTrainConfig
 
       // parse lu content to LUResource object
       let {fileIdToResourceMap: luFileIdToResourceMap, allEmpty: allLuEmpty} = await parseAndValidateContent(luObjectArray, verbose, importResolver, fileExtEnum.LUFile)
 
       // parse qna content to LUResource object
       let {fileIdToResourceMap: qnaFileIdToResourceMap, allEmpty: allQnAEmpty} = await parseAndValidateContent(qnaObjectArray, verbose, importResolver, fileExtEnum.QnAFile)
 
-      if (!allLuEmpty) {
+      if (!allLuEmpty && trainingOpt.intra) {
         // construct resource tree to build the father-children relationship among lu files
         let resources = constructResoureTree(luFileIdToResourceMap, triggerRules)
 
@@ -67,7 +68,7 @@ module.exports = {
         }
       }
 
-      if (!allQnAEmpty) {
+      if (!allQnAEmpty && trainingOpt.inner) {
         // do qna cross training with lu files
         qnaCrossTrain(qnaFileIdToResourceMap, luFileIdToResourceMap, intentName, allLuEmpty)
       }

packages/lu/src/utils/filehelper.ts

@@ -229,7 +229,7 @@ export function getParsedObjects(contents: {id: string, content: string}[], extT
   return parsedObjects
 }
 
-export function getConfigObject(configObject: any, intentName: string, verbose: boolean) {
+export function getConfigObject(configObject: any, intentName: string, verbose: boolean, trainingOpt: {inner: boolean; intra: boolean}) {
   let finalConfigObj = Object.create(null)
   let rootFileIds: string[] = []
   if (configObject) {
@@ -272,7 +272,8 @@ export function getConfigObject(configObject: any, intentName: string, verbose:
     rootIds: rootFileIds,
     triggerRules: finalConfigObj,
     intentName,
-    verbose
+    verbose,
+    trainingOpt
   }
 
   return crossTrainConfig

packages/lu/test/parser/cross-train/crossTrainer.test.js

@@ -199,7 +199,6 @@ describe('luis:cross training tests among lu and qna contents', () => {
     const trainedResult = await crossTrainer.crossTrain(luContentArray, qnaContentArray, configObject)
     const luResult = trainedResult.luResult
     const qnaResult = trainedResult.qnaResult
-
     let foundIndex = luResult.get('Main').Sections.findIndex(s => s.Name === 'DeferToRecognizer_QnA_Main')
     assert.isTrue(foundIndex > -1)
     assert.equal(luResult.get('Main').Sections[foundIndex].Body, `- user guide${NEWLINE}- tell joke`)
@@ -1036,4 +1035,196 @@ describe('luis:cross training tests among lu and qna contents', () => {
     assert.equal(qnaResult.get('main').Sections[foundIndex].Questions.length, 1)
     assert.equal(qnaResult.get('main').Sections[foundIndex].Questions[0], 'book a hotel for me')
   })
+
+  it('luis:cross training can do inner or intra training seperately', async () => {
+    let luContentArray = []
+    let qnaContentArray = []
+
+    luContentArray.push({
+      content:
+        `# dia1_trigger
+        - book a hotel for me
+        
+        # dia2_trigger
+        - book a flight for me
+        - book a train ticket for me`,
+      id: 'Main'})
+
+    qnaContentArray.push({
+      content:
+        `# ?user guide
+
+        **Filters:**
+        - aa=bb
+        
+        \`\`\`
+            Here is the [user guide](http://contoso.com/userguide.pdf)
+        \`\`\`
+        
+        # ?tell joke
+        \`\`\`
+            tell a funny joke
+        \`\`\``,
+      id: 'Main'}
+    )
+
+    luContentArray.push({
+      content:
+        `> !# @app.name = my luis application
+        
+        # hotelLevel
+        - I need a four star hotel
+        
+        # hotelLocation
+        - can I book a hotel near Space Needle`,
+      id: 'Dia1'}
+    )
+
+    qnaContentArray.push({
+      content:
+        `> !# @qna.pair.source = xyz
+        <a id = "1"></a>
+        
+        # ?tell Joke
+        - tell me a joke
+        
+        \`\`\`
+            ha ha ha
+        \`\`\`
+        
+        # ?can I book a hotel near space needle
+        \`\`\`
+            of course you can
+        \`\`\``,
+      id: 'dia1'}
+    )
+
+    luContentArray.push({
+      content:
+        `# dia3_trigger
+        - book a flight from {fromLocation = Seattle} to {toLocation = Beijing}
+        
+        # dia4_trigger
+        - book a train ticket from Seattle to Portland`,
+      id: 'Dia2'}
+    )
+
+    qnaContentArray.push({
+      content:
+        `# ?sing song
+        \`\`\`
+            sure, I can sing song.
+        \`\`\`
+        
+        # ?tell a joke
+        \`\`\`
+            ha ha ha
+        \`\`\``,
+     id: 'dia2'}
+    )
+
+    luContentArray.push({
+      content:
+        `# flightDestination
+        - book a flight to {toLocation = Beijing}
+        
+        # flightTime
+        - which {flightTime} do you prefer`,
+      id: 'dia3'}
+    )
+
+    qnaContentArray.push({
+      content: ``,
+      id: 'dia3'
+    })
+
+    luContentArray.push({
+      content:
+        `# railwayStation
+        - which station will you leave from
+        
+        # railwayTime
+        - when do you want to leave from Seattle train station`,
+      id: 'dia4'})
+
+    qnaContentArray.push({
+      content:
+      `# ? there is only qna for this dialog
+      \`\`\`
+      should add filter meta data here
+      \`\`\``,
+      id: 'dia5'
+    })
+
+    const configObject = {
+      'main': {
+        'rootDialog': true,
+        'triggers': {
+          'dia1_trigger': 'dia1',
+          'dia2_trigger': 'dia2'
+        }
+      },
+      'dia2': {
+        'triggers': {
+          'dia3_trigger': 'dia3',
+          'dia4_trigger': 'dia4'
+        }
+      }
+    }
+
+    const options = {trainingOpt: {inner: true, intra:  false}}
+
+    const trainedResult = await crossTrainer.crossTrain(luContentArray, qnaContentArray, configObject, options)
+    const luResult = trainedResult.luResult
+    const qnaResult = trainedResult.qnaResult
+
+    let luisContent = luResult.get('Main').Content;
+    assert.isTrue(luisContent.includes('DeferToRecognizer_QnA') === true)
+    assert.isTrue(luisContent.includes('_Interruption') === false)
+    
+    luisContent = luResult.get('Dia1').Content;
+    assert.isTrue(luisContent.includes('DeferToRecognizer_QnA') === true)
+    assert.isTrue(luisContent.includes('_Interruption') === false)
+
+    
+
+    luisContent = luResult.get('Dia2').Content;
+    assert.isTrue(luisContent.includes('DeferToRecognizer_QnA') === true)
+    assert.isTrue(luisContent.includes('_Interruption') === false)
+
+    
+    let qnaContent = qnaResult.get('Main').Content;
+    assert.isTrue(qnaContent.includes('DeferToRecognizer_') === true)
+    assert.isTrue(qnaContent.includes('_Interruption') === false)
+
+    qnaContent = qnaResult.get('dia1').Content;
+    assert.isTrue(qnaContent.includes('DeferToRecognizer_') === true)
+    assert.isTrue(qnaContent.includes('_Interruption') === false)
+
+    qnaContent = qnaResult.get('dia2').Content;
+    assert.isTrue(qnaContent.includes('DeferToRecognizer_') === true)
+    assert.isTrue(qnaContent.includes('_Interruption') === false)
+
+    const optionsIntra = {trainingOpt: {inner: false, intra: true}}
+
+    const trainedResult2 = await crossTrainer.crossTrain(luContentArray, qnaContentArray, configObject, optionsIntra)
+    const luResult2 = trainedResult2.luResult
+    const qnaResult2 = trainedResult2.qnaResult
+    
+    luisContent = luResult2.get('Dia1').Content;
+    assert.isTrue(luisContent.includes('# DeferToRecognizer_') === false)
+    assert.isTrue(luisContent.includes('_Interruption') === true)
+
+    luisContent = luResult2.get('Dia2').Content;
+    assert.isTrue(luisContent.includes('# DeferToRecognizer_') === false)
+    assert.isTrue(luisContent.includes('_Interruption') === true)
+
+    qnaContent = qnaResult2.get('dia1').Content;
+    assert.isTrue(qnaContent.includes('DeferToRecognizer_') === false)
+    assert.isTrue(qnaContent.includes('_Interruption') === false)
+
+    qnaContent = qnaResult2.get('dia2').Content;
+    assert.isTrue(qnaContent.includes('DeferToRecognizer_') === false)
+    assert.isTrue(qnaContent.includes('_Interruption') === false)
+  })
 })

packages/luis/README.md

@@ -425,6 +425,11 @@ OPTIONS
   --intentName=intentName  [default: _Interruption] Interruption intent name
 
   --log                    Writes out log messages to console
+
+  --inner-dialog            Only performs the inner dialog cross train, defalt is true, to set it as false, use --no-inner-dialog
+
+  --intra-dialog            Only performs the intra dialog cross train, defalt is true, to set it as false, use --no-intra-dialog
+
 ```
 
 _See code: [src/commands/luis/cross-train.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/luis/src/commands/luis/cross-train.ts)_

packages/luis/src/commands/luis/cross-train.ts

@@ -20,13 +20,14 @@ export default class LuisCrossTrain extends Command {
     config: flags.string({description: 'Path to config file of mapping rules'}),
     intentName: flags.string({description: 'Interruption intent name', default: '_Interruption'}),
     force: flags.boolean({char: 'f', description: 'If --out flag is provided with the path to an existing file, overwrites that file', default: false}),
-    log: flags.boolean({description: 'Writes out log messages to console', default: false})
+    log: flags.boolean({description: 'Writes out log messages to console', default: false}),
+    'inner-dialog': flags.boolean({description: 'Only do inner dialog cross train', default: true, allowNo: true}),
+    'intra-dialog': flags.boolean({description: 'Only do intra dialog cross train', default: true, allowNo: true})
   }
 
   async run() {
     try {
       const {flags} = this.parse(LuisCrossTrain)
-
       if (!flags.in) {
         throw new CLIError('Missing input. Please specify a folder with --in flag')
       }
@@ -39,7 +40,13 @@ export default class LuisCrossTrain extends Command {
         throw new CLIError('Missing cross train config. Please provide config file path by --config.')
       }
 
-      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config, flags.log)
+      let trainingOpt = {}
+      trainingOpt = {
+        inner: flags['inner-dialog'],
+        intra: flags['intra-dialog']
+      }
+
+      const trainedResult = await crossTrain.train(flags.in, flags.intentName, flags.config, flags.log, trainingOpt)
 
       if (flags.out === undefined) {
         flags.out = path.join(process.cwd(), 'cross-trained')

packages/luis/test/commands/luis/crossTrain.test.ts

@@ -87,7 +87,7 @@ describe('luis:cross-train tests for lu and qna contents', () => {
       expect(await compareLuFiles('./../../../interruptionGen/dia2.lu', './../../fixtures/verified/interruption3/dia2.lu')).to.be.true
       expect(await compareLuFiles('./../../../interruptionGen/dia3.lu', './../../fixtures/verified/interruption3/dia3.lu')).to.be.true
     })
-  
+
   test
     .stdout()
     .command(['luis:cross-train',
@@ -114,4 +114,38 @@ describe('luis:cross-train tests for lu and qna contents', () => {
     .it('displays a warning if log is set true', ctx => {
       expect(ctx.stdout).to.contain('[WARN] line 1:0 - line 1:15: no utterances found for intent definition: "# hotelLocation"')
     })
-})
+
+  test
+    .stdout()
+    .command(['luis:cross-train',
+      '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption5')}`,
+      '--intentName', '_Interruption',
+      '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption5/mapping_rules.json')}`,
+      '--out', './interruptionGen',
+      '--force',
+      '--intra-dialog',
+      '--no-inner-dialog'])
+    .it('luis:cross training only do inner dialog', async () => {
+      expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption6/main.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/main.qna', './../../fixtures/verified/interruption6/main.qna')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption6/dia1.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1.qna', './../../fixtures/verified/interruption6/dia1.qna')).to.be.true
+    })
+
+  test
+    .stdout()
+    .command(['luis:cross-train',
+      '--in', `${path.join(__dirname, './../../fixtures/testcases/interruption5')}`,
+      '--intentName', '_Interruption',
+      '--config', `${path.join(__dirname, './../../fixtures/testcases/interruption5/mapping_rules.json')}`,
+      '--out', './interruptionGen',
+      '--force',
+      '--inner-dialog',
+      '--no-intra-dialog'])
+    .it('luis:cross training only do intra dialog', async () => {
+      expect(await compareLuFiles('./../../../interruptionGen/main.lu', './../../fixtures/verified/interruption7/main.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/main.qna', './../../fixtures/verified/interruption7/main.qna')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1.lu', './../../fixtures/verified/interruption7/dia1.lu')).to.be.true
+      expect(await compareLuFiles('./../../../interruptionGen/dia1.qna', './../../fixtures/verified/interruption7/dia1.qna')).to.be.true
+    })
+})