Skip to content

Commit

Permalink
Merge pull request #33 from publicodes/fix-recalcul
Browse files Browse the repository at this point in the history
Fix(optim): don't fold rules modified in `recalcul `
  • Loading branch information
EmileRolley authored Dec 20, 2023
2 parents f27330f + d2368f7 commit 654a65d
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 56 deletions.
129 changes: 73 additions & 56 deletions source/optims/constantFolding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@ type FoldingCtx = {
refs: RefMaps
toKeep?: PredicateOnRule
params: FoldingParams
/**
* The rules that are evaluated with a modified situation (in a [recalcul] mechanism)
* and we don't want to be folded.
*
* @example
* ```
* rule:
* recalcul:
* règle: rule2
* avec:
* rule3: 10
* rule4: 20
* ...
* ```
* In this case, [rule2] should not be folded.
*/
recalculRules: Set<RuleName>
}

function addMapEntry(map: RefMap, key: RuleName, values: RuleName[]) {
Expand All @@ -46,12 +63,22 @@ function initFoldingCtx(
toKeep?: PredicateOnRule,
foldingParams?: FoldingParams,
): FoldingCtx {
const refs: RefMaps = { parents: new Map(), childs: new Map() }
const refs: RefMaps = {
parents: new Map(),
childs: new Map(),
}
const recalculRules = new Set<RuleName>()

Object.entries(parsedRules).forEach(([ruleName, ruleNode]) => {
const reducedAST =
reduceAST(
(acc: Set<RuleName>, node: ASTNode) => {
if (
node.nodeKind === 'recalcul' &&
'dottedName' in node.explanation.recalculNode
) {
recalculRules.add(node.explanation.recalculNode.dottedName)
}
if (
node.nodeKind === 'reference' &&
'dottedName' in node &&
Expand Down Expand Up @@ -80,16 +107,22 @@ function initFoldingCtx(
parsedRules,
refs,
toKeep,
recalculRules,
params: { isFoldedAttr: foldingParams?.isFoldedAttr ?? 'optimized' },
}
}

function isFoldable(rule: RuleNode): boolean {
function isFoldable(
rule: RuleNode | undefined,
recalculRules: Set<RuleName>,
): boolean {
if (!rule) {
return false
}

const rawNode = rule.rawNode
return !(
recalculRules.has(rule.dottedName) ||
'question' in rawNode ||
// NOTE(@EmileRolley): I assume that a rule can have a [par défaut] attribute without a [question] one.
// The behavior could be specified.
Expand All @@ -99,13 +132,6 @@ function isFoldable(rule: RuleNode): boolean {
)
}

function isInParsedRules(
parsedRules: ParsedRules<RuleName>,
rule: RuleName,
): boolean {
return Object.keys(parsedRules).includes(rule)
}

function isEmptyRule(rule: RuleNode): boolean {
// There is always a 'nom' attribute.
return Object.keys(rule.rawNode).length <= 1
Expand Down Expand Up @@ -207,25 +233,23 @@ function searchAndReplaceConstantValueInParentRefs(
ctx: FoldingCtx,
ruleName: RuleName,
rule: RuleNode,
): FoldingCtx {
): void {
const refs = ctx.refs.parents.get(ruleName)

if (refs) {
refs
.map((dottedName) => ctx.parsedRules[dottedName])
.filter(isFoldable)
.forEach((parentRule) => {
const parentName = parentRule.dottedName
for (const parentName of refs) {
let parentRule = ctx.parsedRules[parentName]

if (isFoldable(parentRule, ctx.recalculRules)) {
const newRule = lexicalSubstitutionOfRefValue(parentRule, rule)
if (newRule !== undefined) {
parentRule = newRule
parentRule.rawNode[ctx.params.isFoldedAttr] = true
removeInMap(ctx.refs.parents, ruleName, parentName)
}
})
}
}
}

return ctx
}

function isAlreadyFolded(params: FoldingParams, rule: RuleNode): boolean {
Expand All @@ -237,28 +261,20 @@ function isAlreadyFolded(params: FoldingParams, rule: RuleNode): boolean {
*
* @note It folds child rules in [refs] if possible.
*/
function replaceAllPossibleChildRefs(
ctx: FoldingCtx,
refs: RuleName[],
): FoldingCtx {
function replaceAllPossibleChildRefs(ctx: FoldingCtx, refs: RuleName[]): void {
if (refs) {
refs
.map((dottedName) => ctx.parsedRules[dottedName])
.filter(isFoldable)
.forEach(({ dottedName: childDottedName }) => {
let childNode = ctx.parsedRules[childDottedName]

if (!childNode) {
// TODO: need to investigate
return
}
for (const childName of refs) {
const childNode = ctx.parsedRules[childName]

if (!isAlreadyFolded(ctx.params, childNode)) {
ctx = tryToFoldRule(ctx, childDottedName, childNode)
}
})
if (
childNode &&
isFoldable(childNode, ctx.recalculRules) &&
!isAlreadyFolded(ctx.params, childNode)
) {
tryToFoldRule(ctx, childName, childNode)
}
}
}
return ctx
}

export function removeInMap<K, V>(
Expand All @@ -278,48 +294,47 @@ function removeRuleFromRefs(ref: RefMap, ruleName: RuleName) {
})
}

function deleteRule(ctx: FoldingCtx, dottedName: RuleName): FoldingCtx {
function deleteRule(ctx: FoldingCtx, dottedName: RuleName): void {
const ruleNode = ctx.parsedRules[dottedName]
if (
(ctx.toKeep === undefined || !ctx.toKeep([dottedName, ruleNode])) &&
isFoldable(ruleNode)
isFoldable(ruleNode, ctx.recalculRules)
) {
removeRuleFromRefs(ctx.refs.parents, dottedName)
removeRuleFromRefs(ctx.refs.childs, dottedName)
delete ctx.parsedRules[dottedName]
ctx.refs.parents.delete(dottedName)
ctx.refs.childs.delete(dottedName)
}
return ctx
}

/** Removes the [parentRuleName] as a parent dependency of each [childRuleNamesToUpdate]. */
function updateRefCounting(
ctx: FoldingCtx,
parentRuleName: RuleName,
ruleNamesToUpdate: RuleName[],
): FoldingCtx {
): void {
ruleNamesToUpdate.forEach((ruleNameToUpdate) => {
removeInMap(ctx.refs.parents, ruleNameToUpdate, parentRuleName)
if (ctx.refs.parents.get(ruleNameToUpdate)?.length === 0) {
ctx = deleteRule(ctx, ruleNameToUpdate)
deleteRule(ctx, ruleNameToUpdate)
}
})
return ctx
}

function tryToFoldRule(
ctx: FoldingCtx,
ruleName: RuleName,
rule: RuleNode,
): FoldingCtx {
): void {
if (
rule !== undefined &&
(isAlreadyFolded(ctx.params, rule) ||
!isInParsedRules(ctx.parsedRules, ruleName))
(!isFoldable(rule, ctx.recalculRules) ||
isAlreadyFolded(ctx.params, rule) ||
!(ruleName in ctx.parsedRules))
) {
// Already managed rule
return ctx
return
}

const ruleParents = ctx.refs.parents.get(ruleName)
Expand All @@ -329,7 +344,7 @@ function tryToFoldRule(
) {
// Empty rule with no parent
deleteRule(ctx, ruleName)
return ctx
return
}

const { nodeValue, missingVariables, traversedVariables, unit } =
Expand Down Expand Up @@ -360,17 +375,17 @@ function tryToFoldRule(
)
}

ctx = searchAndReplaceConstantValueInParentRefs(ctx, ruleName, rule)
searchAndReplaceConstantValueInParentRefs(ctx, ruleName, rule)
if (ctx.parsedRules[ruleName] === undefined) {
return ctx
return
}

if ('formule' in rule.rawNode) {
// The rule do not depends on any other rule anymore, so we need to remove
// it from the [refs].
const childs = ctx.refs.childs.get(ruleName) ?? []

ctx = updateRefCounting(
updateRefCounting(
ctx,
ruleName,
// NOTE(@EmileRolley): for some reason, the [traversedVariables] are not always
Expand All @@ -392,15 +407,14 @@ function tryToFoldRule(
ctx.parsedRules[ruleName].rawNode[ctx.params.isFoldedAttr] = true
}

return ctx
return
} else if ('formule' in rule.rawNode) {
// Try to replace internal refs if possible.
const childs = ctx.refs.childs.get(ruleName)
if (childs?.length > 0) {
replaceAllPossibleChildRefs(ctx, childs)
}
}
return ctx
}

/**
Expand All @@ -425,8 +439,11 @@ export function constantFolding(
let ctx: FoldingCtx = initFoldingCtx(engine, parsedRules, toKeep, params)

Object.entries(ctx.parsedRules).forEach(([ruleName, ruleNode]) => {
if (isFoldable(ruleNode) && !isAlreadyFolded(ctx.params, ruleNode)) {
ctx = tryToFoldRule(ctx, ruleName, ruleNode)
if (
isFoldable(ruleNode, ctx.recalculRules) &&
!isAlreadyFolded(ctx.params, ruleNode)
) {
tryToFoldRule(ctx, ruleName, ruleNode)
}
})

Expand All @@ -435,7 +452,7 @@ export function constantFolding(
Object.entries(ctx.parsedRules).filter(([ruleName, ruleNode]) => {
const parents = ctx.refs.parents.get(ruleName)
return (
!isFoldable(ruleNode) ||
!isFoldable(ruleNode, ctx.recalculRules) ||
toKeep([ruleName, ruleNode]) ||
parents?.length > 0
)
Expand Down
80 changes: 80 additions & 0 deletions test/optims/constantFolding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,85 @@ describe('Constant folding [base]', () => {
})
})

it('should not fold rules used in a [recalcul]', () => {
const rawRules = {
root: {
recalcul: {
règle: 'rule to recompute',
avec: {
constant: 20,
},
},
},
'rule to recompute': {
formule: 'constant * 2',
},
constant: {
valeur: 10,
},
}
expect(constantFoldingWith(rawRules)).toStrictEqual({
root: {
recalcul: {
règle: 'rule to recompute',
avec: {
constant: 20,
},
},
},
'rule to recompute': {
formule: 'constant * 2',
},
constant: {
valeur: 10,
optimized: true,
},
})
})

it('should not fold rules used in a [recalcul] but still fold used constant in other rules', () => {
const rawRules = {
root: {
recalcul: {
règle: 'rule to recompute',
avec: {
constant: 20,
},
},
},
'rule to recompute': {
formule: 'constant * 2',
},
'rule to fold': {
formule: 'constant * 4',
},
constant: {
valeur: 10,
},
}
expect(constantFoldingWith(rawRules)).toStrictEqual({
root: {
recalcul: {
règle: 'rule to recompute',
avec: {
constant: 20,
},
},
},
'rule to recompute': {
formule: 'constant * 2',
},
'rule to fold': {
valeur: '40',
optimized: true,
},
constant: {
valeur: 10,
optimized: true,
},
})
})

it('replaceAllRefs bug #3', () => {
const rawRules = {
boisson: {
Expand All @@ -561,6 +640,7 @@ describe('Constant folding [base]', () => {
},
})
})

//
//
// TODO: not supported yet
Expand Down

0 comments on commit 654a65d

Please sign in to comment.