Skip to content

Add randomesque #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jakub-jedrusiak wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add randomesque #46

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8464b0e
Add randomesque
jakub-jedrusiak Feb 5, 2026
629564e
Adapt to catR more closely
jakub-jedrusiak Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions src/cat.ts
View file
Open in desktop

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest blocker for me right now is that none of the changes are tested. Could you add test coverage for:

  • randomesque = 1 (default) returns the deterministic top-1 pick for both mfi and closest.
  • randomesque = 3 with a fixed randomSeed: the picked item is always one of the top 3 by Fisher information; over many seeds, all 3 candidates appear.
  • Same scenario for closest.
  • Tie inclusion: construct stimuli with a 3-way tie at the K-th rank with randomesque = 2, assert the random pool is 3 items.
  • Empty stimuli returns { nextStimulus: undefined, remainingStimuli: [] } for both selectors.
  • Reproducibility: two Cat instances with the same randomSeed and randomesque > 1 produce the same pick sequence.
  • Validation: new Cat({ randomesque: 0 }), -1, 2.5, and NaN all throw errors. (Requires the suggestion I made about validating the input.)

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { minimize_Powell } from 'optimization-js';
import { Stimulus, Zeta } from './type';
import { itemResponseFunction, fisherInformation, normal, uniform, findClosest } from './utils';
import { itemResponseFunction, fisherInformation, normal, uniform } from './utils';
import { validateZetaParams, fillZetaDefaults, ensureZetaNumericValues } from './corpus';
import seedrandom from 'seedrandom';
import _clamp from 'lodash/clamp';
Expand All @@ -18,6 +18,7 @@ export interface CatInput {
priorDist?: string;
priorPar?: number[];
randomSeed?: string | null;
randomesque?: number;
}

export class Cat {
Expand All @@ -27,6 +28,7 @@ export class Cat {
public maxTheta: number;
public priorDist: string;
public priorPar: number[];
public randomesque: number;
private readonly _zetas: Zeta[];
private readonly _resps: (0 | 1)[];
private _theta: number;
Expand All @@ -38,7 +40,7 @@ export class Cat {

/**
* Create a Cat object. This expects an single object parameter with the following keys
* @param {{method: string, itemSelect: string, nStartItems: number, startSelect:string, theta: number, minTheta: number, maxTheta: number, priorDist: string, priorPar: number[]}=} destructuredParam
* @param {{method: string, itemSelect: string, nStartItems: number, startSelect:string, theta: number, minTheta: number, maxTheta: number, priorDist: string, priorPar: number[], randomSeed: string|null, randomesque: number}=} destructuredParam
* method: ability estimator, e.g. MLE or EAP, default = 'MLE'
* itemSelect: the method of item selection, e.g. "MFI", "random", "closest", default method = 'MFI'

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* itemSelect: the method of item selection, e.g. "MFI", "random", "closest", default method = 'MFI'
* itemSelect: the method of item selection, e.g. "MFI", "random", "closest", default method = 'MFI'. "MFI" and "closest" additionally honor the `randomesque` parameter.

* nStartItems: first n trials to keep non-adaptive selection
Expand All @@ -49,6 +51,7 @@ export class Cat {
* priorDist: the prior distribution type (only applies to EAP estimator)
* priorPar: the prior distribution parameters (only applies to EAP estimator)
* randomSeed: set a random seed to trace the simulation
* randomesque: number of top items to randomly select from in MFI/closest (default = 1, i.e. always pick the best)

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* randomesque: number of top items to randomly select from in MFI/closest (default = 1, i.e. always pick the best)
* randomesque: when selecting an item via 'MFI' or 'closest', randomly pick from the
top N candidates rather than always taking the best. Use values > 1 to introduce
controlled variability and reduce item exposure across examinees, at a small cost
to measurement efficiency. Items tied at the N-th rank are all included in the
random pool, so the effective pool size may exceed `randomesque` when ties exist.
This mirrors catR's `randomesque` behavior. Must be a
positive integer; throws if invalid. Default = 1 (always pick the best).

*/

constructor({
Expand All @@ -62,6 +65,7 @@ export class Cat {
priorDist = 'norm', // only applies to EAP estimator
priorPar = priorDist === 'unif' ? [-4, 4] : [0, 1], // only applies to EAP estimator
randomSeed = null,
randomesque = 1,
}: CatInput = {}) {
this.method = Cat.validateMethod(method);

Expand All @@ -78,6 +82,7 @@ export class Cat {
this._theta = theta;
this._seMeasurement = Number.MAX_VALUE;
this.nStartItems = nStartItems;
this.randomesque = Math.max(1, Math.round(randomesque));

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This quietly turns 0, -3, 2.7, NaN, etc. into something "valid," which is inconsistent with how every other CatInput option is handled, e.g., validateMethod, validateItemSelect, validateStartSelect, and validatePrior all throw on bad input. Could you add a Cat.validateRandomesque static (or inline check) that throws on non-positive integers and non-numbers? Something like:

private static validateRandomesque(randomesque: number): number {
  if (!Number.isInteger(randomesque) || randomesque < 1) {
    throw new Error(
      `randomesque must be a positive integer. Received ${randomesque}.`,
    );
  }
  return randomesque;
}

this._rng = randomSeed === null ? seedrandom() : seedrandom(randomSeed);
this._prior = this.method === 'eap' ? Cat.validatePrior(priorDist, priorPar, minTheta, maxTheta) : [];
}
Expand Down Expand Up @@ -291,13 +296,28 @@ export class Cat {
...element,
}));

if (stimuliAddFisher.length === 0) {
return { nextStimulus: undefined as unknown as Stimulus, remainingStimuli: [] as Stimulus[] };

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double-cast hides a real type contract. findNextItem already returned undefined for nextStimulus in the empty-stimuli case before this PR (the test at cat.test.ts:286-287 relies on it), so the actual return type has always been { nextStimulus: Stimulus | undefined; remainingStimuli: Stimulus[] }. The new explicit guards are an improvement, but the casts paper over a pre-existing latent typing bug. Concrete fix:

  1. Declare a return-type alias. Add after CatInput closes at cat.ts:22:
export interface SelectorResult {
  nextStimulus: Stimulus | undefined;
  remainingStimuli: Stimulus[];
}
  1. Annotate findNextItem. cat.ts:257, change:
public findNextItem(stimuli: Stimulus[], itemSelect: string = this.itemSelect, deepCopy = true) {

to:

public findNextItem(
  stimuli: Stimulus[],
  itemSelect: string = this.itemSelect,
  deepCopy = true,
): SelectorResult {

  1. Annotate the private selectors. Same : SelectorResult annotation on selectorMFI (cat.ts:292), selectorClosest (cat.ts:340), selectorMiddle (cat.ts:324), selectorRandom (cat.ts:363), and selectorFixed (cat.ts:381). The latter three already implicitly return Stimulus | undefined for nextStimulus (e.g. arr.shift() on an empty array, arr[index] when index is out of bounds), so the annotation just makes the existing contract explicit.

  2. Drop the casts in selectorMFI. cat.ts:299-301, replace:

    if (stimuliAddFisher.length === 0) {
      return { nextStimulus: undefined as unknown as Stimulus, remainingStimuli: [] as Stimulus[] };
    }

with:

    if (stimuliAddFisher.length === 0) {
      return { nextStimulus: undefined, remainingStimuli: [] };
    }

5.. Drop the casts in selectorClosest. cat.ts:342-344, same change:

    if (arr.length === 0) {
      return { nextStimulus: undefined, remainingStimuli: [] };
    }
  1. Drop the cast on picked.** cat.ts:319, change:
      nextStimulus: picked as Stimulus,

to:

      nextStimulus: picked,

The as Stimulus was only there to satisfy the previous (incorrect) return shape; once SelectorResult.nextStimulus is Stimulus | undefined, picked (which is Stimulus) widens cleanly.

  1. Update the JSDoc. cat.ts:255, change:
@returns {nextStimulus: Stimulus, remainingStimuli: Array<Stimulus>}

to:

@returns {nextStimulus: Stimulus | undefined, remainingStimuli: Array<Stimulus>}

After these changes, tsc --noEmit should be clean and the call site in clowder.ts:410 (const { nextStimulus } = cat.findNextItem(...)) will correctly type nextStimulus as Stimulus | undefined, which matches how Clowder already treats it downstream.

}

stimuliAddFisher.sort((a, b) => b.fisherInformation - a.fisherInformation);
stimuliAddFisher.forEach((stimulus: Stimulus) => {

// Randomesque: pick randomly from the top-K items by Fisher information
const nrIt = Math.min(this.randomesque, stimuliAddFisher.length);
const threshold = stimuliAddFisher[nrIt - 1].fisherInformation;
const topK = stimuliAddFisher.filter((s) => s.fisherInformation >= threshold);

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in selectorClosest about the thresholding behavior.

Also see the comment there about extracting this logic to a shared utility.

const pickIndex = this.randomInteger(0, topK.length - 1);
const picked = topK[pickIndex];

const remaining = stimuliAddFisher.filter((s) => s !== picked);
remaining.forEach((stimulus: Stimulus) => {
delete stimulus['fisherInformation'];
});
delete (picked as Stimulus)['fisherInformation'];
Comment on lines +312 to +316

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code deleted fisherInformation from every element in one pass. We can keep that:

stimuliAddFisher.forEach((s) => delete (s as Stimulus)['fisherInformation']);
const remaining = stimuliAddFisher.filter((s) => s !== picked);


return {
nextStimulus: stimuliAddFisher[0],
remainingStimuli: stimuliAddFisher.slice(1).sort((a: Stimulus, b: Stimulus) => a.difficulty! - b.difficulty!),
nextStimulus: picked as Stimulus,
remainingStimuli: remaining.sort((a: Stimulus, b: Stimulus) => a.difficulty! - b.difficulty!),
};
}

Expand All @@ -319,9 +339,21 @@ export class Cat {

private selectorClosest(arr: Stimulus[]) {

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Suggested change
private selectorClosest(arr: Stimulus[]) {
/**
* Picks the item whose difficulty is closest to a target value derived from
* the current ability estimate (see the note on the `0.481` offset below).
* When `randomesque > 1`, randomly picks from the K nearest items rather than
* always the closest. Items tied at the K-th-nearest distance are all included
* in the random pool, so the effective pool size may exceed `randomesque` when
* ties exist (this matches catR's `randomesque` semantics in the `bOpt` rule).
*
* @param arr - The list of stimuli to choose from.
* @returns {Object} - An object with the next item and the updated list.
* @returns {Stimulus | undefined} return.nextStimulus - The picked item, or
* `undefined` if `arr` is empty.
* @returns {Stimulus[]} return.remainingStimuli - The remaining stimuli, in
* the order they had on input (with the picked item removed).
*/
private selectorClosest(arr: Stimulus[]) {

//findClosest requires arr is sorted by difficulty
const index = findClosest(arr, this._theta + 0.481);
const nextItem = arr[index];
arr.splice(index, 1);
if (arr.length === 0) {
return { nextStimulus: undefined as unknown as Stimulus, remainingStimuli: [] as Stimulus[] };

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about the double-cast.

}
// Compute distances from target for randomesque support
const target = this._theta + 0.481;
const distances = arr.map((s, i) => ({ index: i, dist: Math.abs(s.difficulty! - target) }));
distances.sort((a, b) => a.dist - b.dist);

const nrIt = Math.min(this.randomesque, distances.length);
const threshold = distances[nrIt - 1].dist;
const topK = distances.filter((d) => d.dist <= threshold);

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The >= threshold / <= threshold filters mean that when items tie at the K-th rank, all of them are included in the random pool. E.g., with randomesque = 2 and Fisher info [5, 3, 3, 3, 1], the pool is the three 3s, not two of them. I think this is intentional. Does catR do the same thing? If it's intentional, please add a comment explaining it to the reader so that the next developer doesn't try to come in and "fix" it.

const pick = topK[this.randomInteger(0, topK.length - 1)];
Comment on lines +350 to +353

@richford richford May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectorMFI and selectorClosest now have the same threshold/filter/randomInteger pattern. Could you extract a small helper, e.g.:

private pickFromTopK<T>(sorted: T[], score: (item: T) => number): T {
  const nrIt = Math.min(this.randomesque, sorted.length);
  const threshold = score(sorted[nrIt - 1]);
  const topK = sorted.filter((s) => score(s) >= threshold);
  // For closest, score returns *distance*, so the comparator flips.
  // pass an `order: "desc" | "asc"` flag, or have the caller pre-negate.
  return topK[this.randomInteger(0, topK.length - 1)];
}

If we ever revisit the tie semantics it'll only need to change in one place.


const nextItem = arr[pick.index];
arr.splice(pick.index, 1);
return {
nextStimulus: nextItem,
remainingStimuli: arr,
Expand Down