Skip to content

Commit

Permalink
Merge pull request #898 from concord-consortium/185896734-case-index
Browse files Browse the repository at this point in the history 
Evaluate `caseIndex` in formulas
  • Loading branch information
@pjanik
pjanik authored Sep 27, 2023
2 parents b435b5a + 1b8a4c4 commit 505ee9d
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 29 deletions.
7 changes: 5 additions & 2 deletions v3/src/models/data/formula-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from "./formula-utils"
import {
DisplayNameMap, IFormulaDependency, GLOBAL_VALUE, LOCAL_ATTR, ILocalAttributeDependency, IGlobalValueDependency,
ILookupDependency, NO_PARENT_KEY, FValue
ILookupDependency, NO_PARENT_KEY, FValue, CASE_INDEX_FAKE_ATTR_ID
} from "./formula-types"
import { math } from "./formula-fn-registry"
import { IDataSet } from "./data-set"
Expand Down Expand Up @@ -355,7 +355,10 @@ export class FormulaManager {
}

displayNameMap.localNames = {
...mapAttributeNames(localDataSet, LOCAL_ATTR)
...mapAttributeNames(localDataSet, LOCAL_ATTR),
// caseIndex is a special name supported by formulas. It essentially behaves like a local data set attribute
// that returns the current, 1-based index of the case in its collection group.
caseIndex: `${LOCAL_ATTR}${CASE_INDEX_FAKE_ATTR_ID}`
}

this.globalValueManager?.globals.forEach(global => {
Expand Down
57 changes: 43 additions & 14 deletions v3/src/models/data/formula-mathjs-scope.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { AGGREGATE_SYMBOL_SUFFIX, FValue, GLOBAL_VALUE, LOCAL_ATTR, NO_PARENT_KEY } from "./formula-types"
import {
FValue, AGGREGATE_SYMBOL_SUFFIX, CASE_INDEX_FAKE_ATTR_ID, GLOBAL_VALUE, LOCAL_ATTR, NO_PARENT_KEY
} from "./formula-types"
import type { IGlobalValueManager } from "../global/global-value-manager"
import type { IDataSet } from "./data-set"
import type { IValueType } from "./attribute"
Expand All @@ -23,6 +25,8 @@ export class FormulaMathJsScope {
context: IFormulaMathjsScopeContext
caseId = ""
dataStorage: Record<string, any> = {}
caseIndexCache?: Record<string, number>
// `cache` is used directly by custom formula functions like `prev`, `next` or other aggregate functions.
cache = new Map<string, any>()
// Previous result is used for calculating recursive functions like prev() referencing itself, e.g.:
// prev(CumulativeValue, 0) + Value
Expand All @@ -36,53 +40,55 @@ export class FormulaMathJsScope {
}

initDataStorage(context: IFormulaMathjsScopeContext) {
// `caseIndex` is a special symbol that might be used by formulas.
const localAttributeIds = context.localDataSet.attributes.map(a => a.id).concat(CASE_INDEX_FAKE_ATTR_ID)

// We could parse symbol name in get() function, but this should theoretically be faster, as it's done only once,
// and no parsing is needed when the symbol is accessed for each dataset case.
// First, provide local dataset attribute symbols.
context.localDataSet.attributes.forEach(attr => {
Object.defineProperty(this.dataStorage, `${LOCAL_ATTR}${attr.id}`, {
localAttributeIds.forEach(attrId => {
Object.defineProperty(this.dataStorage, `${LOCAL_ATTR}${attrId}`, {
get: () => {
return context.localDataSet.getValue(this.caseId, attr.id)
return this.getLocalValue(this.caseId, attrId)
}
})

// Make sure that all the caching and case processing is done lazily, only for attributes that are actually
// referenced by the formula.
const cachedGroup: Record<string, IValueType[]> = {}
let cacheInitialized = false
Object.defineProperty(this.dataStorage, `${LOCAL_ATTR}${attr.id}${AGGREGATE_SYMBOL_SUFFIX}`, {
let cachedGroup: Record<string, IValueType[]>
Object.defineProperty(this.dataStorage, `${LOCAL_ATTR}${attrId}${AGGREGATE_SYMBOL_SUFFIX}`, {
get: () => {
if (this.usePreviousCase) {
// Note that this block is only used by `prev()` function that has iterative approach to calculating
// its values rather than relying on arrays of values like other aggregate functions. However, its arguments
// are still considered aggregate, so caching and grouping works as expected.
if (attr.id === this.context.formulaAttrId) {
if (attrId === this.context.formulaAttrId) {
// When formula references its own attribute, we cannot simply return case values - we're just trying
// to calculate them. In most cases this is not allowed, but there are some exceptions, e.g. prev function
// referencing its own attribute. It could be used to calculate cumulative value in a recursive way.
return this.previousResult
}
return context.localDataSet.getValue(this.previousCaseId, attr.id)
return this.getLocalValue(this.previousCaseId, attrId)
}

if (!cacheInitialized) {
if (!cachedGroup) {
cachedGroup = {}
// Cache is calculated lazily to avoid calculating it for all the attributes that are not referenced by
// the formula. Note that each case is processed only once, so this mapping is only O(n) complexity.
context.childMostCollectionCases.forEach(c => {
const groupId = context.caseGroupId[c.__id__]
if (!cachedGroup[groupId]) {
cachedGroup[groupId] = []
}
cachedGroup[groupId].push(context.localDataSet.getValue(c.__id__, attr.id))
cachedGroup[groupId].push(this.getLocalValue(c.__id__, attrId))
})
cacheInitialized = true
}
return cachedGroup[this.getCaseGroupId()] || cachedGroup[NO_PARENT_KEY]
}
})
})

// Then, provide global value symbols.
// Global value symbols.
context.globalValueManager?.globals.forEach(global => {
Object.defineProperty(this.dataStorage, `${GLOBAL_VALUE}${global.id}`, {
get: () => {
Expand All @@ -91,7 +97,6 @@ export class FormulaMathJsScope {
})
})
}

// --- Functions required by MathJS scope "interface". It doesn't seem to be defined/typed anywhere, so it's all
// based on: // https://github.com/josdejong/mathjs/blob/develop/examples/advanced/custom_scope_objects.js ---
get(key: string): any {
Expand Down Expand Up @@ -127,6 +132,30 @@ export class FormulaMathJsScope {
}

// --- Custom functions used by our formulas or formula manager --
getCaseIndex(caseId: string) {
if (!this.caseIndexCache) {
// Cache is calculated lazily to avoid calculating when not necessary.
// Note that each case is processed only once, so this mapping is only O(n) complexity.
this.caseIndexCache = {}
const casesCount: Record<string, number> = {}
this.context.childMostCollectionCases.forEach(c => {
const groupId = this.context.caseGroupId[c.__id__]
if (!casesCount[groupId]) {
casesCount[groupId] = 0
}
casesCount[groupId] += 1
this.caseIndexCache![c.__id__] = casesCount[groupId]
})
}
return this.caseIndexCache[caseId]
}

getLocalValue(caseId: string, attrId: string) {
return attrId === CASE_INDEX_FAKE_ATTR_ID
? this.getCaseIndex(caseId)
: this.context.localDataSet.getValue(caseId, attrId)
}

setCaseId(caseId: string) {
this.caseId = caseId
}
Expand Down
1 change: 1 addition & 0 deletions v3/src/models/data/formula-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { FormulaMathJsScope } from "./formula-mathjs-scope"
export const GLOBAL_VALUE = "GLOBAL_VALUE_"
export const LOCAL_ATTR = "LOCAL_ATTR_"
export const AGGREGATE_SYMBOL_SUFFIX = "_ALL"
export const CASE_INDEX_FAKE_ATTR_ID = "CASE_INDEX"

export const NO_PARENT_KEY = "__NO_PARENT__"

Expand Down
29 changes: 16 additions & 13 deletions v3/src/models/data/formula-utils.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { parse, MathNode, isFunctionNode, isSymbolNode } from "mathjs"
import {
AGGREGATE_SYMBOL_SUFFIX, LOCAL_ATTR, GLOBAL_VALUE, DisplayNameMap, IFormulaDependency, ILocalAttributeDependency,
AGGREGATE_SYMBOL_SUFFIX, LOCAL_ATTR, GLOBAL_VALUE, DisplayNameMap, IFormulaDependency, ILocalAttributeDependency
} from "./formula-types"
import { typedFnRegistry } from "./formula-fn-registry"
import type { IDataSet } from "./data-set"
Expand All @@ -27,7 +27,7 @@ export const generateCanonicalSymbolName = (name: string, aggregate: boolean, di
return canonicalName
}

export const parseCanonicalSymbolName = (canonicalName: string): IFormulaDependency | null => {
export const parseCanonicalSymbolName = (canonicalName: string): IFormulaDependency | undefined => {
if (canonicalName.startsWith(LOCAL_ATTR)) {
const attrId = canonicalName.substring(LOCAL_ATTR.length)
const result: ILocalAttributeDependency = { type: "localAttribute", attrId }
Expand All @@ -41,7 +41,7 @@ export const parseCanonicalSymbolName = (canonicalName: string): IFormulaDepende
const globalId = canonicalName.substring(GLOBAL_VALUE.length)
return { type: "globalValue", globalId }
}
return null
return undefined
}

export const safeSymbolName = (name: string) => {
Expand All @@ -62,6 +62,9 @@ export const customizeFormula = (formula: string) => {
.replace(/`([^`]+)`/g, (_, match) => safeSymbolName(match))
}

export const ifSelfReference = (dependency?: IFormulaDependency, formulaAttributeId?: string) =>
formulaAttributeId && dependency?.type === "localAttribute" && dependency.attrId === formulaAttributeId

// Function replaces all the symbol names typed by user (display names) with the symbol canonical names that
// can be resolved by formula context and do not rely on user-based display names.
export const canonicalizeExpression = (displayExpression: string, displayNameMap: DisplayNameMap) => {
Expand Down Expand Up @@ -130,13 +133,12 @@ export const getFormulaDependencies = (formulaCanonical: string, formulaAttribut
}
const isSelfReferenceAllowed = !!node.isSelfReferenceAllowed
if (isSymbolNode(node)) {
const parsedName = parseCanonicalSymbolName(node.name)
const isNodeReferencingItself = formulaAttributeId &&
parsedName?.type === "localAttribute" && parsedName.attrId === formulaAttributeId
const dependency = parseCanonicalSymbolName(node.name)
const isSelfReference = ifSelfReference(dependency, formulaAttributeId)
// Note that when self reference is allowed, we should NOT add the attribute to the dependency list.
// This would create cycle in observers and trigger an error even earlier, when we check for this scenario.
if (parsedName && (!isNodeReferencingItself || !isSelfReferenceAllowed)) {
result.push(parsedName)
if (dependency && (!isSelfReference || !isSelfReferenceAllowed)) {
result.push(dependency)
}
}
// Some functions have special kind of dependencies that need to be calculated in a custom way
Expand Down Expand Up @@ -180,15 +182,16 @@ export const getExtremeCollectionDependency =
export const getFormulaChildMostAggregateCollectionIndex = (formulaCanonical: string, dataSet: IDataSet) => {
const attrId = getExtremeCollectionDependency(formulaCanonical, dataSet, { order: "max", aggregate: true })
const collectionId = dataSet.getCollectionForAttribute(attrId || "")?.id
return dataSet.getCollectionIndex(collectionId || "") ?? null
const collectionIndex = dataSet.getCollectionIndex(collectionId || "")
return collectionIndex !== -1 ? dataSet.getCollectionIndex(collectionId || "") : null
}

export const getIncorrectParentAttrReference =
(formulaCanonical: string, formulaCollectionIndex: number, dataSet: IDataSet) => {
const attrId = getExtremeCollectionDependency(formulaCanonical, dataSet, { order: "min", aggregate: true })
const collectionId = dataSet.getCollectionForAttribute(attrId || "")?.id
const collectionIndex = dataSet.getCollectionIndex(collectionId || "") ?? Infinity
if (collectionIndex < formulaCollectionIndex) {
const collectionIndex = dataSet.getCollectionIndex(collectionId || "")
if (collectionIndex !== -1 && collectionIndex < formulaCollectionIndex) {
return attrId
}
return false
Expand All @@ -198,8 +201,8 @@ export const getIncorrectChildAttrReference =
(formulaCanonical: string, formulaCollectionIndex: number, dataSet: IDataSet) => {
const attrId = getExtremeCollectionDependency(formulaCanonical, dataSet, { order: "max", aggregate: false })
const collectionId = dataSet.getCollectionForAttribute(attrId || "")?.id
const collectionIndex = dataSet.getCollectionIndex(collectionId || "") ?? -Infinity
if (collectionIndex > formulaCollectionIndex) {
const collectionIndex = dataSet.getCollectionIndex(collectionId || "")
if (collectionIndex !== -1 && collectionIndex > formulaCollectionIndex) {
return attrId
}
return false
Expand Down

0 comments on commit 505ee9d

Please sign in to comment.