Skip to content

Commit

Permalink
Merge pull request #900 from concord-consortium/186090743-prev-prev
Browse files Browse the repository at this point in the history
Formulas can evaluate nested `prev` calls (e.g. prev(fib, 1) + prev(prev(fib, 1), 1))
  • Loading branch information
pjanik authored Sep 27, 2023
2 parents 34c6061 + 4a00b0f commit b7a79f6
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
4 changes: 2 additions & 2 deletions v3/src/models/data/formula-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,14 @@ export class FormulaManager {
dataSet.setCaseValues(casesToRecalculate.map((c) => {
// This is necessary for functions like `prev` that need to know the previous result when they reference
// its own attribute.
formulaScope.setPreviousCaseId(formulaScope.getCaseId())
formulaScope.savePreviousCaseId(formulaScope.getCaseId())
formulaScope.setCaseId(c.__id__)
let formulaValue: FValue
try {
formulaValue = compiledFormula.evaluate(formulaScope)
// This is necessary for functions like `prev` that need to know the previous result when they reference
// its own attribute.
formulaScope.setPreviousResult(formulaValue)
formulaScope.savePreviousResult(formulaValue)
} catch (e: any) {
formulaValue = formulaError(e.message)
}
Expand Down
27 changes: 14 additions & 13 deletions v3/src/models/data/formula-mathjs-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ export class FormulaMathJsScope {
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.:
// Properties defined below are used for calculating recursive functions like prev() referencing itself, e.g.:
// prev(CumulativeValue, 0) + Value
usePreviousCase = false
previousResult: FValue = ""
previousCaseId = ""
previousCaseIdxModifier = 0
previousResults: FValue[] = []
previousCaseIds: string[] = []

constructor (context: IFormulaMathjsScopeContext) {
this.context = context
Expand All @@ -58,17 +58,18 @@ export class FormulaMathJsScope {
let cachedGroup: Record<string, IValueType[]>
Object.defineProperty(this.dataStorage, `${LOCAL_ATTR}${attrId}${AGGREGATE_SYMBOL_SUFFIX}`, {
get: () => {
if (this.usePreviousCase) {
if (this.previousCaseIdxModifier !== 0) {
// 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 (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 this.previousResults[this.previousResults.length + this.previousCaseIdxModifier]
}
return this.getLocalValue(this.previousCaseId, attrId)
const prevCaseId = this.previousCaseIds[this.previousCaseIds.length + this.previousCaseIdxModifier]
return this.getLocalValue(prevCaseId, attrId)
}

if (!cachedGroup) {
Expand Down Expand Up @@ -164,18 +165,18 @@ export class FormulaMathJsScope {
return this.caseId
}

setPreviousCaseId(caseId: string) {
this.previousCaseId = caseId
savePreviousCaseId(caseId: string) {
this.previousCaseIds.push(caseId)
}

setPreviousResult(value: FValue) {
this.previousResult = value
savePreviousResult(value: FValue) {
this.previousResults.push(value)
}

withPreviousCase(callback: () => void) {
this.usePreviousCase = true
this.previousCaseIdxModifier -= 1
callback()
this.usePreviousCase = false
this.previousCaseIdxModifier += 1
}

getCaseChildrenCount() {
Expand Down

0 comments on commit b7a79f6

Please sign in to comment.