Skip to content

Commit

Permalink
Fixed a minor bug where min / max values weren't being respected in m…
Browse files Browse the repository at this point in the history
…c sequence generation. Removed the offset and updated unit tests to catch this issue in the future.
  • Loading branch information
abrisene committed Sep 7, 2021
1 parent 3a4052e commit 426bc5b
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 54 deletions.
41 changes: 0 additions & 41 deletions examples/basic copy.js

This file was deleted.

4 changes: 2 additions & 2 deletions examples/mc.basic.js → examples/markov.basic.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
# examples/basic.ts
# Basic Example
# examples/mc.basic.ts
# Markov Chain Basic Example
*/

// import { MarkovChain } from '..';
Expand Down
2 changes: 1 addition & 1 deletion examples/markov.howto.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
# examples/dist.howto.js
# examples/markov.howto.js
# Distribution Howto Example
*/

Expand Down
43 changes: 39 additions & 4 deletions src/__tests__/markov.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,12 @@ function validateInstance(m: MarkovChain, ref = defaultDTO) {
expect(m.maxOrder).toEqual(ref.maxOrder);
expect(m.delimiter).toEqual(ref.delimiter);
expect(m.startDelimiter).toEqual(ref.startDelimiter);
// expect(m.seed).toEqual(ref.seed);
// expect(m.uses).toEqual(ref.uses);
expect(m).toHaveProperty('sequences');
expect(m).toHaveProperty('grams');
expect(m).toHaveProperty('seed');
expect(m).toHaveProperty('uses');
validateDTO(data, ref);
}

Expand Down Expand Up @@ -98,16 +102,25 @@ function validateGrams(m: MarkovChainDTO) {
function validateGen(model: MarkovChainDTO, output: string[], options: MCGeneratorOptions = defaultGenOptions) {
expect(output).toBeDefined(); // If we're testing this, we expect it to be defined.
if (output !== undefined) {
expect(output.length).toBeGreaterThan(options.min || defaultGenOptions.min);
expect(output.length).toBeLessThan(options.max || defaultGenOptions.max);
expect(output.length).toBeGreaterThanOrEqual(options.min || defaultGenOptions.min);
// expect(output.length).toBeLessThanOrEqual(options.max || defaultGenOptions.max);

if (options.trim) {
if (options.trim === true) {
expect(output.length).toBeLessThanOrEqual(options.max || defaultGenOptions.max);
expect(output.filter(v => v === model.startDelimiter || v === model.endDelimiter).length).toEqual(0);
} else {
} else if (options.trim === false) {
expect(output.length).toBeLessThanOrEqual(options.max ? options.max + 1 : defaultGenOptions.max + 1);
// expect(output.filter(v => (v === model.startDelimiter || v === model.endDelimiter)).length).toBeGreaterThanOrEqual(1);
expect(output.filter(v => v === model.startDelimiter || v === model.endDelimiter).length).toBeLessThanOrEqual(2);
}
}

/* if (options.min) expect(output.length).toBeGreaterThanOrEqual(options.min);
if (options.max && options.trim) {
expect(output.length).toBeLessThanOrEqual(options.max);
} else if (options.max && !options.trim) {
expect(output.length).toBeLessThanOrEqual(options.max + 2);
} */
}

/**
Expand Down Expand Up @@ -469,6 +482,28 @@ describe('Markov Chain', () => {
validateGen(dtoA3, genD2, optD2);
validateGen(dtoA3, genD3, optD3);

// Min and Max
const optM0: MCGeneratorStaticOptions = { model: dtoA3, min: 2, trim: true };
const optM1: MCGeneratorStaticOptions = { model: dtoA3, min: 2, trim: false };
const optM2: MCGeneratorStaticOptions = { model: dtoA3, max: 2, trim: true };
const optM3: MCGeneratorStaticOptions = { model: dtoA3, max: 2, trim: false };
const optM4: MCGeneratorStaticOptions = { model: dtoA3, min: 2, max: 2, trim: true };
const optM5: MCGeneratorStaticOptions = { model: dtoA3, min: 2, max: 2, trim: false };

const genM0 = MarkovChain.generate(optM0);
const genM1 = MarkovChain.generate(optM1);
const genM2 = MarkovChain.generate(optM2);
const genM3 = MarkovChain.generate(optM3);
const genM4 = MarkovChain.generate(optM4);
const genM5 = MarkovChain.generate(optM5);

validateGen(dtoA3, genM0, optM0);
validateGen(dtoA3, genM1, optM1);
validateGen(dtoA3, genM2, optM2);
validateGen(dtoA3, genM3, optM3);
validateGen(dtoA3, genM4, optM4);
validateGen(dtoA3, genM5, optM5);

// Starting Values
const optS1: MCGeneratorStaticOptions = { model: dtoA3, engine: eng, start: ['a', 'n'] };
const optS2: MCGeneratorStaticOptions = { model: dtoA3, engine: eng, start: ['n', 'a'], direction: 'last' };
Expand Down
13 changes: 7 additions & 6 deletions src/structures/markov.ts
Original file line number Diff line number Diff line change
Expand Up @@ -788,23 +788,24 @@ export class MarkovChain {
let curOrder = start !== undefined ? start.length : 1;

// Determine the offset for our picks.
// TODO: There's an issue with this which leads to too many picks.
const pickOffset = trim ? 2 : 0;
const minPicks = min + pickOffset;
const maxPicks = max + pickOffset;
// Removed this because it wasn't necessary and causing a bug.
// TODO: Keep as a reminder and then remove if there aren't any long term issues.
// const pickOffset = trim ? 0 : 0;
// const minPicks = min + pickOffset;
// const maxPicks = max + pickOffset;

// Determine the temporary mask to use while sequence is less than min.
const tempMask = mask !== undefined ? [terminator, ...mask] : [terminator];

// Utility function for finding the current sequence given order and direction.

// MAKE THE PICKS
for (let i = 0; picks.length <= maxPicks; i += 1) {
for (let i = 0; picks.length <= max; i += 1) {
// Increase the order if we're below the desired value.
if (curOrder < maxOrder) curOrder += 1;

// Determine which mask we should use.
const pickMask = picks.length < minPicks ? tempMask : mask;
const pickMask = picks.length < min ? tempMask : mask;

// Find the gram
const gram = strict
Expand Down

0 comments on commit 426bc5b

Please sign in to comment.