Skip to content
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.

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

pkg: upgrade publicodes to 1.0.0 (NGC-454) #34

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"license": "MIT",
"dependencies": {
"@types/node": "^18.11.18",
"publicodes": "^1.0.0-beta.77"
"publicodes": "^1.0.0-rc.7"
},
"devDependencies": {
"@types/jest": "^29.2.5",
Expand Down
4 changes: 2 additions & 2 deletions source/commons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export type ImportMacro = {
/**
* Represents a non-parsed NGC rule.
*/
export type RawRule = Omit<Rule, 'nom'> & ImportMacro
export type RawRule = Omit<Rule, 'nom'> | ImportMacro

/**
* Represents a non-parsed NGC model.
Expand Down Expand Up @@ -115,7 +115,7 @@ export const disabledLogger: Logger = {
*
* @returns The references.
*/
export function getAllRefsInNode(node: RuleNode): RuleName[] {
export function getAllRefsInNode(node: ASTNode): RuleName[] {
return reduceAST<RuleName[]>(
(refs: RuleName[], node: ASTNode) => {
if (node === undefined) {
Expand Down
22 changes: 2 additions & 20 deletions source/compilation/resolveImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,21 +162,6 @@ ${importedRule.description}`
}
}

/**
* @throws {Error} If the `nom` attribute is different from the `ruleNameToCheck`.
*/
function removeRawNodeNom(
rawNode: Rule,
ruleNameToCheck: string,
): Omit<Rule, 'nom'> {
const { nom, ...rest } = rawNode
if (nom !== ruleNameToCheck)
throw Error(
`Imported rule's publicode raw node "nom" attribute is different from the resolveImport script ruleName. Please investigate`,
)
return rest
}

function appearsMoreThanOnce(
rulesToImport: RuleToImport[],
ruleName: RuleName,
Expand Down Expand Up @@ -218,7 +203,7 @@ export function resolveImports(
let neededNamespaces = new Set<string>()
const resolvedRules = Object.entries(rules).reduce((acc, [name, value]) => {
if (name === IMPORT_KEYWORD) {
const importMacro: ImportMacro = value
const importMacro = value as ImportMacro
const engine = getEngine(filePath, importMacro, verbose)
const rulesToImport: RuleToImport[] =
importMacro['les règles']?.map(getRuleToImportInfos)
Expand Down Expand Up @@ -252,10 +237,7 @@ export function resolveImports(
utils
.ruleParents(ruleName)
.forEach((rule) => neededNamespaces.add(`${namespace} . ${rule}`))
return [
`${namespace} . ${ruleName}`,
removeRawNodeNom(ruleWithUpdatedDescription, ruleName),
]
return [`${namespace} . ${ruleName}`, ruleWithUpdatedDescription]
}

const ruleWithOverridenAttributes = { ...rule.rawNode, ...attrs }
Expand Down
115 changes: 71 additions & 44 deletions source/optims/constantFolding.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Engine, { reduceAST, ParsedRules, parseExpression } from 'publicodes'
import type { RuleNode, ASTNode, Unit } from 'publicodes'
import {
getAllRefsInNode,
RuleName,
serializeParsedExprAST,
substituteInParsedExpr,
Expand Down Expand Up @@ -31,22 +32,28 @@ type FoldingCtx = {
toKeep?: PredicateOnRule
params: FoldingParams
/**
* The rules that are evaluated with a modified situation (in a [recalcul] mechanism)
* The rules that are evaluated with a modified situation (in a [contexte] mechanism)
* and we don't want to be folded.
*
* @example
* ```
* rule:
* recalcul:
* règle: rule2
* avec:
* rule3: 10
* rule4: 20
* valeur: rule2
* contexte:
* rule3: 10
* rule4: 20
* ...
* ```
* In this case, [rule2] should not be folded.
* In this case, [rule2] should not be folded (and all its dependencies
* should not be folded!).
*
* TODO(@EmileRolley): currently, all childs of a rule with a [contexte]
* mechanism are not folded. However, it could be smarter to keep track of
* each contexte rules and fold the child rules that are not impacted by the
* contexte. For now we choose to keep it simple and to over-fold instead of
* taking the risk to alter the result.
*/
recalculRules: Set<RuleName>
impactedByContexteRules: Set<RuleName>
}

function addMapEntry(map: RefMap, key: RuleName, values: RuleName[]) {
Expand All @@ -67,17 +74,22 @@ function initFoldingCtx(
parents: new Map(),
childs: new Map(),
}
const recalculRules = new Set<RuleName>()
const impactedByContexteRules = 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 === 'contexte') {
// Find all rule references impacted by the contexte in the rule node
const impactedRules = getAllRefsInNodeImpactedByContexte(
ruleName,
node,
node.explanation.contexte.map(([ref, _]) => ref.dottedName),
)

impactedRules.forEach((rule) => impactedByContexteRules.add(rule))
impactedByContexteRules.add(ruleName)
}
if (
node.nodeKind === 'reference' &&
Expand All @@ -90,6 +102,7 @@ function initFoldingCtx(
new Set(),
ruleNode.explanation.valeur,
) ?? new Set()

const traversedVariables: RuleName[] = Array.from(reducedAST).filter(
(name) => !name.endsWith(' . $SITUATION'),
)
Expand All @@ -102,27 +115,58 @@ function initFoldingCtx(
}
})

// All childs of a rule impacted by a contexte rule are also impacted.
//
// NOTE(@EmileRolley): contexte rule will be added in the contextRules set.
// Therefore, they won't be marked as folded. It's a wanted behavior? Not sure.
//
// WARN(@EmileRolley): the [impactedByContexteRules] is updated while
// iterating it's convenient but the semantics may vary depending on the
// javascript runtime used.
for (const ruleName of impactedByContexteRules) {
refs.childs
.get(ruleName)
?.forEach((rule) => impactedByContexteRules.add(rule))
}

return {
engine,
parsedRules,
refs,
toKeep,
recalculRules,
impactedByContexteRules,
params: { isFoldedAttr: foldingParams?.isFoldedAttr ?? 'optimized' },
}
}

function getAllRefsInNodeImpactedByContexte(
ruleName: RuleName,
node: ASTNode,
contexteRefs: RuleName[],
): RuleName[] {
const impactedRules = getAllRefsInNode(node).filter((ref) => {
return (
ref !== ruleName &&
!ref.endsWith(' . $SITUATION') &&
!contexteRefs.includes(ref)
)
})

return impactedRules
}

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

const rawNode = rule.rawNode

return !(
recalculRules.has(rule.dottedName) ||
contextRules.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 @@ -133,8 +177,7 @@ function isFoldable(
}

function isEmptyRule(rule: RuleNode): boolean {
// There is always a 'nom' attribute.
return Object.keys(rule.rawNode).length <= 1
return Object.keys(rule.rawNode).length === 0
}

function formatPublicodesUnit(unit?: Unit): string {
Expand Down Expand Up @@ -240,7 +283,7 @@ function searchAndReplaceConstantValueInParentRefs(
for (const parentName of refs) {
let parentRule = ctx.parsedRules[parentName]

if (isFoldable(parentRule, ctx.recalculRules)) {
if (isFoldable(parentRule, ctx.impactedByContexteRules)) {
const newRule = lexicalSubstitutionOfRefValue(parentRule, rule)
if (newRule !== undefined) {
parentRule = newRule
Expand Down Expand Up @@ -268,7 +311,7 @@ function replaceAllPossibleChildRefs(ctx: FoldingCtx, refs: RuleName[]): void {

if (
childNode &&
isFoldable(childNode, ctx.recalculRules) &&
isFoldable(childNode, ctx.impactedByContexteRules) &&
!isAlreadyFolded(ctx.params, childNode)
) {
tryToFoldRule(ctx, childName, childNode)
Expand Down Expand Up @@ -298,7 +341,7 @@ function deleteRule(ctx: FoldingCtx, dottedName: RuleName): void {
const ruleNode = ctx.parsedRules[dottedName]
if (
(ctx.toKeep === undefined || !ctx.toKeep([dottedName, ruleNode])) &&
isFoldable(ruleNode, ctx.recalculRules)
isFoldable(ruleNode, ctx.impactedByContexteRules)
) {
removeRuleFromRefs(ctx.refs.parents, dottedName)
removeRuleFromRefs(ctx.refs.childs, dottedName)
Expand Down Expand Up @@ -329,7 +372,7 @@ function tryToFoldRule(
): void {
if (
rule !== undefined &&
(!isFoldable(rule, ctx.recalculRules) ||
(!isFoldable(rule, ctx.impactedByContexteRules) ||
isAlreadyFolded(ctx.params, rule) ||
!(ruleName in ctx.parsedRules))
) {
Expand All @@ -347,16 +390,9 @@ function tryToFoldRule(
return
}

const { nodeValue, missingVariables, traversedVariables, unit } =
ctx.engine.evaluateNode(rule)

const traversedVariablesWithoutSelf = traversedVariables.filter(
(dottedName) => dottedName !== ruleName,
)
const { nodeValue, missingVariables, unit } = ctx.engine.evaluateNode(rule)

// NOTE(@EmileRolley): we need to evaluate due to possible standalone rule [formule]
// parsed as a [valeur].
if ('valeur' in rule.rawNode && traversedVariablesWithoutSelf?.length > 0) {
if ('valeur' in rule.rawNode) {
rule.rawNode.formule = rule.rawNode.valeur
delete rule.rawNode.valeur
}
Expand Down Expand Up @@ -385,16 +421,7 @@ function tryToFoldRule(
// it from the [refs].
const childs = ctx.refs.childs.get(ruleName) ?? []

updateRefCounting(
ctx,
ruleName,
// NOTE(@EmileRolley): for some reason, the [traversedVariables] are not always
// depencies of the rule. Consequently, we need to keep only the ones that are
// in the [childs] list in order to avoid removing rules that are not dependencies.
traversedVariablesWithoutSelf?.filter((v: RuleName) =>
childs.includes(v),
) ?? [],
)
updateRefCounting(ctx, ruleName, childs)
delete ctx.parsedRules[ruleName].rawNode.formule
}

Expand Down Expand Up @@ -440,7 +467,7 @@ export function constantFolding(

Object.entries(ctx.parsedRules).forEach(([ruleName, ruleNode]) => {
if (
isFoldable(ruleNode, ctx.recalculRules) &&
isFoldable(ruleNode, ctx.impactedByContexteRules) &&
!isAlreadyFolded(ctx.params, ruleNode)
) {
tryToFoldRule(ctx, ruleName, ruleNode)
Expand All @@ -452,7 +479,7 @@ export function constantFolding(
Object.entries(ctx.parsedRules).filter(([ruleName, ruleNode]) => {
const parents = ctx.refs.parents.get(ruleName)
return (
!isFoldable(ruleNode, ctx.recalculRules) ||
!isFoldable(ruleNode, ctx.impactedByContexteRules) ||
toKeep([ruleName, ruleNode]) ||
parents?.length > 0
)
Expand Down
3 changes: 3 additions & 0 deletions test/getRawRules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('getRawRules', () => {
titre: 'Rule A',
formule: 'B . C * 3',
},
'ruleA . B': null,
'ruleA . B . C': {
valeur: '10',
},
Expand All @@ -51,6 +52,7 @@ describe('getRawRules', () => {
titre: 'Rule A',
formule: 'B . C * 3',
},
'ruleA . B': null,
'ruleA . B . C': {
valeur: '10',
},
Expand All @@ -60,6 +62,7 @@ describe('getRawRules', () => {
titre: 'Rule A',
formule: 'B . C * 3',
},
'ruleA . B': null,
'ruleA . B . C': {
valeur: '10',
},
Expand Down
Loading
Loading