Skip to content

Commit

Permalink
Move scorer: Interactive Graph (#2120)
Browse files Browse the repository at this point in the history
* STOPSHIP some type errors still

* add back duplicate declarations

* add back Line duplicate

* all tests passing

* Revert changes to underscore's isEqual (#2125)

## Summary:
[Original comment](#2113 (comment))

I made a separate PR because I made this mistake in a couple of PRs so I thought I'd knock them out all at once.

Issue: LEMS-2737

## Test plan:

Author: handeyeco

Reviewers: jeremywiebe, benchristel

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ❌ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️  [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️  [cancelled] Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2125

* respond to Jeremy's feedback
  • Loading branch information
handeyeco authored Jan 17, 2025
1 parent 58ebb49 commit 5c83869
Show file tree
Hide file tree
Showing 41 changed files with 359 additions and 274 deletions.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
import {clockwise} from "../../../util/geometry";
// This file contains helper functions for working with angles.

import type {Coord} from "@khanacademy/perseus";
import type {vec} from "mafs";
import {clockwise} from "./geometry";

// This file contains helper functions for working with angles.
import type {Coord} from "@khanacademy/perseus-core";

export function convertDegreesToRadians(degrees: number): number {
return (degrees / 180) * Math.PI;
}

// Returns a value between -180 and 180, inclusive. The angle is measured
// between the positive x-axis and the given vector.
export function calculateAngleInDegrees([x, y]: vec.Vector2): number {
export function calculateAngleInDegrees([x, y]: Coord): number {
return (Math.atan2(y, x) * 180) / Math.PI;
}

// Converts polar coordinates to cartesian. The th(eta) parameter is in degrees.
export function polar(r: number | vec.Vector2, th: number): vec.Vector2 {
export function polar(r: number | Coord, th: number): Coord {
if (typeof r === "number") {
r = [r, r];
}
Expand All @@ -26,10 +25,7 @@ export function polar(r: number | vec.Vector2, th: number): vec.Vector2 {
// This function calculates the angle between two points and an optional vertex.
// If the vertex is not provided, the angle is measured between the two points.
// This does not account for reflex angles or clockwise position.
export const getAngleFromVertex = (
point: vec.Vector2,
vertex: vec.Vector2,
): number => {
export const getAngleFromVertex = (point: Coord, vertex: Coord): number => {
const x = point[0] - vertex[0];
const y = point[1] - vertex[1];
if (!x && !y) {
Expand Down
62 changes: 62 additions & 0 deletions packages/kmath/src/coefficients.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type {SineCoefficient} from "./geometry";
import type {Coord} from "@khanacademy/perseus-core";

export type NamedSineCoefficient = {
amplitude: number;
angularFrequency: number;
phase: number;
verticalOffset: number;
};

// TODO: there's another, very similar getSinusoidCoefficients function
// they should probably be merged
export function getSinusoidCoefficients(
coords: ReadonlyArray<Coord>,
): SineCoefficient {
// It's assumed that p1 is the root and p2 is the first peak
const p1 = coords[0];
const p2 = coords[1];

// Resulting coefficients are canonical for this sine curve
const amplitude = p2[1] - p1[1];
const angularFrequency = Math.PI / (2 * (p2[0] - p1[0]));
const phase = p1[0] * angularFrequency;
const verticalOffset = p1[1];

return [amplitude, angularFrequency, phase, verticalOffset];
}

export type QuadraticCoefficient = [number, number, number];

// TODO: there's another, very similar getQuadraticCoefficients function
// they should probably be merged
export function getQuadraticCoefficients(
coords: ReadonlyArray<Coord>,
): QuadraticCoefficient {
const p1 = coords[0];
const p2 = coords[1];
const p3 = coords[2];

const denom = (p1[0] - p2[0]) * (p1[0] - p3[0]) * (p2[0] - p3[0]);
if (denom === 0) {
// Many of the callers assume that the return value is always defined.
// @ts-expect-error - TS2322 - Type 'undefined' is not assignable to type 'QuadraticCoefficient'.
return;
}
const a =
(p3[0] * (p2[1] - p1[1]) +
p2[0] * (p1[1] - p3[1]) +
p1[0] * (p3[1] - p2[1])) /
denom;
const b =
(p3[0] * p3[0] * (p1[1] - p2[1]) +
p2[0] * p2[0] * (p3[1] - p1[1]) +
p1[0] * p1[0] * (p2[1] - p3[1])) /
denom;
const c =
(p2[0] * p3[0] * (p2[0] - p3[0]) * p1[1] +
p3[0] * p1[0] * (p3[0] - p1[0]) * p2[1] +
p1[0] * p2[0] * (p1[0] - p2[0]) * p3[1]) /
denom;
return [a, b, c];
}
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
* A collection of geomtry-related utility functions
*/

import {number as knumber, point as kpoint, sum} from "@khanacademy/kmath";
import {
approximateDeepEqual,
approximateEqual,
type Coord,
} from "@khanacademy/perseus-core";
import _ from "underscore";

import Util from "../util";

import type {Coord, Line} from "../interactive2/types";
import {number as knumber, point as kpoint, sum} from "@khanacademy/kmath";

const {eq, deepEq} = Util;
type Line = [Coord, Coord];

// This should really be a readonly tuple of [number, number]
export type Range = [number, number];
Expand All @@ -21,12 +23,9 @@ export type SineCoefficient = [
number, // verticalOffset
];

// a, b, c
export type QuadraticCoefficient = [number, number, number];

// Given a number, return whether it is positive (1), negative (-1), or zero (0)
export function sign(val: number): 0 | 1 | -1 {
if (eq(val, 0)) {
if (approximateEqual(val, 0)) {
return 0;
}
return val > 0 ? 1 : -1;
Expand All @@ -39,7 +38,7 @@ export function ccw(a: Coord, b: Coord, c: Coord): number {
}

export function collinear(a: Coord, b: Coord, c: Coord): boolean {
return eq(ccw(a, b, c), 0);
return approximateEqual(ccw(a, b, c), 0);
}

// Given rect bounding points A and B, whether point C is inside the rect
Expand Down Expand Up @@ -229,15 +228,15 @@ export function similar(
// @ts-expect-error - TS4104 - The type 'readonly number[]' is 'readonly' and cannot be assigned to the mutable type 'number[]'.
sides = rotate(sides, i);

if (deepEq(angles1, angles)) {
if (approximateDeepEqual(angles1, angles)) {
const sidePairs = _.zip(sides1, sides);

const factors = _.map(sidePairs, function (pair) {
return pair[0] / pair[1];
});

const same = _.all(factors, function (factor) {
return eq(factors[0], factor);
return approximateEqual(factors[0], factor);
});

const congruentEnough = _.all(sidePairs, function (pair) {
Expand Down Expand Up @@ -304,7 +303,7 @@ export function rotate<T>(
}

export function getLineEquation(first: Coord, second: Coord): string {
if (eq(first[0], second[0])) {
if (approximateEqual(first[0], second[0])) {
return "x = " + first[0].toFixed(3);
}
const m = (second[1] - first[1]) / (second[0] - first[0]);
Expand Down
8 changes: 8 additions & 0 deletions packages/kmath/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,13 @@ export * as vector from "./vector";
export * as point from "./point";
export * as line from "./line";
export * as ray from "./ray";
export * as angles from "./angles";
export * as geometry from "./geometry";
export * as coefficients from "./coefficients";

export {default as KhanMath, sum} from "./math";

export type {Range, SineCoefficient} from "./geometry";
export type {NamedSineCoefficient, QuadraticCoefficient} from "./coefficients";

export type * from "./types";
3 changes: 3 additions & 0 deletions packages/kmath/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type {Coord} from "@khanacademy/perseus-core";

export type QuadraticCoords = [Coord, Coord, Coord];
2 changes: 2 additions & 0 deletions packages/perseus-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export type {Coords} from "./utils/grapher-types";
export {addLibraryVersionToPerseusDebug} from "./utils/add-library-version-to-perseus-debug";
export {default as getMatrixSize} from "./utils/get-matrix-size";
export {default as getDecimalSeparator} from "./utils/get-decimal-separator";
export {approximateEqual, approximateDeepEqual} from "./utils/equality";
export {default as deepClone} from "./utils/deep-clone";
export * as GrapherUtil from "./utils/grapher-util";

export {libVersion} from "./version";
Expand Down
25 changes: 25 additions & 0 deletions packages/perseus-core/src/utils/deep-clone.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import deepClone from "./deep-clone";

describe("deepClone", () => {
it("does nothing to a primitive", () => {
expect(deepClone(3)).toBe(3);
});

it("copies an array", () => {
const input = [1, 2, 3];

const result = deepClone(input);

expect(result).toEqual(input);
expect(result).not.toBe(input);
});

it("recursively clones array elements", () => {
const input = [[1]];

const result = deepClone(input);

expect(result).toEqual(input);
expect(result[0]).not.toBe(input[0]);
});
});
18 changes: 18 additions & 0 deletions packages/perseus-core/src/utils/deep-clone.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// TODO(benchristel): in the future, we may want to make deepClone work for
// Record<string, Cloneable> as well. Currently, it only does arrays.
type Cloneable =
| null
| undefined
| boolean
| string
| number
| Cloneable[]
| readonly Cloneable[];
function deepClone<T extends Cloneable>(obj: T): T {
if (Array.isArray(obj)) {
return obj.map(deepClone) as T;
}
return obj;
}

export default deepClone;
55 changes: 55 additions & 0 deletions packages/perseus-core/src/utils/equality.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import _ from "underscore";

/**
* APPROXIMATE equality on numbers and primitives.
*/
export function approximateEqual<T>(x: T, y: T): boolean {
if (typeof x === "number" && typeof y === "number") {
return Math.abs(x - y) < 1e-9;
}
return x === y;
}

/**
* Deep APPROXIMATE equality on primitives, numbers, arrays, and objects.
* Recursive.
*/
export function approximateDeepEqual<T>(x: T, y: T): boolean {
if (Array.isArray(x) && Array.isArray(y)) {
if (x.length !== y.length) {
return false;
}
for (let i = 0; i < x.length; i++) {
if (!approximateDeepEqual(x[i], y[i])) {
return false;
}
}
return true;
}
if (Array.isArray(x) || Array.isArray(y)) {
return false;
}
if (typeof x === "function" && typeof y === "function") {
return approximateEqual(x, y);
}
if (typeof x === "function" || typeof y === "function") {
return false;
}
if (typeof x === "object" && typeof y === "object" && !!x && !!y) {
return (
x === y ||
(_.all(x, function (v, k) {
// @ts-expect-error - TS2536 - Type 'CollectionKey<T>' cannot be used to index type 'T'.
return approximateDeepEqual(y[k], v);
}) &&
_.all(y, function (v, k) {
// @ts-expect-error - TS2536 - Type 'CollectionKey<T>' cannot be used to index type 'T'.
return approximateDeepEqual(x[k], v);
}))
);
}
if ((typeof x === "object" && !!x) || (typeof y === "object" && !!y)) {
return false;
}
return approximateEqual(x, y);
}
8 changes: 5 additions & 3 deletions packages/perseus-core/src/utils/grapher-util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import _ from "underscore";

import {approximateDeepEqual} from "./equality";

import type {
LinearType,
QuadraticType,
Expand Down Expand Up @@ -95,7 +97,7 @@ const PlotDefaults = {
coeffs1: ReadonlyArray<number>,
coeffs2: ReadonlyArray<number>,
): boolean {
return _.isEqual(coeffs1, coeffs2);
return approximateDeepEqual(coeffs1, coeffs2);
},
movable: MOVABLES.PLOT,
getPropsForCoeffs: function (coeffs: ReadonlyArray<number>): {fn: any} {
Expand Down Expand Up @@ -269,7 +271,7 @@ const Sinusoid: SinusoidType = _.extend({}, PlotDefaults, {
coeffs1: ReadonlyArray<number>,
coeffs2: ReadonlyArray<number>,
) {
return _.isEqual(
return approximateDeepEqual(
canonicalSineCoefficients(coeffs1),
canonicalSineCoefficients(coeffs2),
);
Expand Down Expand Up @@ -326,7 +328,7 @@ const Tangent: TangentType = _.extend({}, PlotDefaults, {
coeffs1: ReadonlyArray<number>,
coeffs2: ReadonlyArray<number>,
) {
return _.isEqual(
return approximateDeepEqual(
canonicalTangentCoefficients(coeffs1),
canonicalTangentCoefficients(coeffs2),
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {vector as kvector} from "@khanacademy/kmath";
import {angles, vector as kvector} from "@khanacademy/kmath";
import {
getAngleCoords,
getCircleCoords,
Expand All @@ -9,13 +9,14 @@ import {
getQuadraticCoords,
getSegmentCoords,
getSinusoidCoords,
getClockwiseAngle,
} from "@khanacademy/perseus";
import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core";

import type {StartCoords} from "./types";
import type {Range, PerseusGraphType, Coord} from "@khanacademy/perseus-core";

const {getClockwiseAngle} = angles;

export function getStartCoords(graph: PerseusGraphType): StartCoords {
if ("startCoords" in graph) {
return graph.startCoords;
Expand Down
1 change: 1 addition & 0 deletions packages/perseus-score/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export {default as scoreDropdown} from "./widgets/dropdown/score-dropdown";
export {default as scoreExpression} from "./widgets/expression/score-expression";
export {default as scoreGrapher} from "./widgets/grapher/score-grapher";
export {default as scoreIframe} from "./widgets/iframe/score-iframe";
export {default as scoreInteractiveGraph} from "./widgets/interactive-graph/score-interactive-graph";
export {
default as scoreLabelImage,
labelImageScoreMarker,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import invariant from "tiny-invariant";

import {clone} from "../../../../../testing/object-utils";
import _ from "underscore";

import scoreInteractiveGraph from "./score-interactive-graph";

import type {PerseusInteractiveGraphRubric} from "../../validation.types";
import type {PerseusGraphType} from "@khanacademy/perseus-core";
import type {PerseusInteractiveGraphRubric} from "@khanacademy/perseus-score";

describe("InteractiveGraph scoring on a segment question", () => {
it("marks the answer invalid if guess.coords is missing", () => {
Expand Down Expand Up @@ -326,7 +325,7 @@ describe("InteractiveGraph scoring on a point question", () => {
},
};

const guessClone = clone(guess);
const guessClone = _.clone(guess);

scoreInteractiveGraph(guess, rubric);

Expand All @@ -352,7 +351,7 @@ describe("InteractiveGraph scoring on a point question", () => {
},
};

const rubricClone = clone(rubric);
const rubricClone = _.clone(rubric);

scoreInteractiveGraph(guess, rubric);

Expand Down
Loading

0 comments on commit 5c83869

Please sign in to comment.