From d705e7e047647aa208d2cd0efdc7211014503413 Mon Sep 17 00:00:00 2001 From: Alexandre Hajjar Date: Thu, 1 Oct 2020 12:15:43 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A7=20WIP=20-=20Building=20the=20cycle?= =?UTF-8?q?s=20graph=20by=20taking=20into=20account=20parent=20rule?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This contextualization will allow to have a (more complex) graph that will contain the information of the parent rule of the current rule. This will allow calling `getApplicableReplacedBy` and thus remove the flattening logic, which was imperfect. On the other hand, this needs to make recursive calls to `ruleDepsOfRule` in case of a reference node, and thus make the graph much fatter. Approx TODO (see `ruleDependencies.ts`): - [ ] refactor to propagate the `parentRuleNode` in the rule dependencies - [ ] build recursive calls into `ruleDepsOfReference` --- publicodes/source/cyclesLib/graph.ts | 101 +----------------- .../source/cyclesLib/rulesDependencies.ts | 26 +++-- publicodes/source/parseReference.js | 1 - publicodes/test/cycles.test.js | 79 +------------- 4 files changed, 26 insertions(+), 181 deletions(-) diff --git a/publicodes/source/cyclesLib/graph.ts b/publicodes/source/cyclesLib/graph.ts index 91a204603..4d3cc08ce 100644 --- a/publicodes/source/cyclesLib/graph.ts +++ b/publicodes/source/cyclesLib/graph.ts @@ -12,7 +12,7 @@ type GraphCycles = Array> export class GraphError extends Error {} -export function buildNaiveDependenciesGraph( +export function buildDependenciesGraph( rulesDeps: RulesDependencies, assertNoMultiDependency = false ): graphlib.Graph { @@ -44,110 +44,15 @@ export function buildNaiveDependenciesGraph( return g } - -/** - * If a part of a graph is like: - * A -formule-> B - * B -replacedBy-> A - * (what is called a "remplace one-level loop" or "ROLL") - * then split it in two separated sub-graphs - * Af -formule-> Bf - * Br -replacedBy-> Ar - * and re-plug the other edges related to A and B on Af, Bf, Ar and Br. - * - * This operation is destroying the initial loop, and corresponds closely to - * the behavior of the `parseReference.js:getApplicableReplacedBy` function. - */ -export function flattenOneLevelRemplaceLoops(naiveGraph: graphlib.Graph) { - const replacedByEdges = naiveGraph - .edges() - .filter(e => naiveGraph.edge(e).type == DependencyType.replacedBy) - const ROLLEdges = replacedByEdges.flatMap(e => { - // Note: there is at max one such reverse edge, because graphlib doesn't - // store more than one edge with the same in and out. - const reverseEdges = naiveGraph.inEdges(e.v, e.w) - if ( - reverseEdges.length > 0 && - naiveGraph.edge(reverseEdges[0]).type == DependencyType.formule - ) { - return [e, reverseEdges[0]] - } else { - return [] - } - }) - - // Map with default Set values: - const ROLLNodesTypes = new Proxy( - {}, - { - get: (target, name) => - name in target ? target[name] : (target[name] = new Set()) - } - ) - ROLLEdges.forEach(e => { - ROLLNodesTypes[e.v].add(naiveGraph.edge(e).type) - ROLLNodesTypes[e.w].add(naiveGraph.edge(e).type) - }) - - const flattenedGraph = new graphlib.Graph() - - const specifyNodeName = (depType, nodeName) => - `${nodeName} [depType: ${depType}]` - - naiveGraph.edges().forEach(e => { - // Nodes which are forming the one-level remplace loop: duplicate them so as - // to have 2 independent sub-graphs: one representing the formule - // dependency, the other representing the remplace dependency. - if (ROLLEdges.includes(e)) { - const edgeType = naiveGraph.edge(e).type - flattenedGraph.setEdge( - specifyNodeName(edgeType, e.v), - specifyNodeName(edgeType, e.w), - { type: edgeType } - ) - } - - // Edges which are incoming or outgoing of the loop nodes: duplicate them on - // sub-graphs. - // Outgoing: - else if (e.v in ROLLNodesTypes) { - ROLLNodesTypes[e.v].forEach(depType => { - flattenedGraph.setEdge(specifyNodeName(depType, e.v), e.w, { - type: naiveGraph.edge(e).type - }) - }) - } - // Incoming: - else if (e.w in ROLLNodesTypes) { - // [XXX] To remove: - if (e.v in ROLLNodesTypes) { - throw new Error('shouldnt happen') - } - ROLLNodesTypes[e.w].forEach(depType => { - flattenedGraph.setEdge(e.v, specifyNodeName(depType, e.w), { - type: naiveGraph.edge(e).type - }) - }) - } - - // Edges which are un-related: just copy them. - else { - flattenedGraph.setEdge(e.v, e.w, { type: naiveGraph.edge(e).type }) - } - }) - return flattenedGraph -} - export function cyclicDependencies( rawRules: Rules | string, assertNoMultiDependency = false ): GraphCycles { const parsedRules = parseRules(rawRules) const rulesDependencies = buildRulesDependencies(parsedRules) - const naiveGraph = buildNaiveDependenciesGraph( + const dependenciesGraph = buildDependenciesGraph( rulesDependencies, assertNoMultiDependency ) - const flattenedGraph = flattenOneLevelRemplaceLoops(naiveGraph) - return graphlib.alg.findCycles(flattenedGraph) + return graphlib.alg.findCycles(dependenciesGraph) } diff --git a/publicodes/source/cyclesLib/rulesDependencies.ts b/publicodes/source/cyclesLib/rulesDependencies.ts index 7392148c5..33689fead 100644 --- a/publicodes/source/cyclesLib/rulesDependencies.ts +++ b/publicodes/source/cyclesLib/rulesDependencies.ts @@ -1,5 +1,6 @@ import * as R from 'ramda' import { ParsedRules } from '../types' +import { getApplicableReplacedBy } from '../parseReference' import * as ASTTypes from './ASTTypes' export enum DependencyType { @@ -64,6 +65,16 @@ export function ruleDepsOfNode( function ruleDepsOfReference( reference: ASTTypes.Reference ): RuleDependencies { + // [XXX] Todo: + // - build a context stack (before) and pass it over in ruleDepsOfNode and + // this sub-function + // - take a look at all other sub-functions like RecalculMech to check if + // they need the same treatment + // - modify the graph node labels: hash(ruleName, context stack) + // - here, push the (current) ruleName to the stack + // - call ruleDepsOfRuleNode on (reference.dottedName, newStack) + // - rewire accordingly + // - and indeed in the function make the call to getApplicableReplacedBy return [[reference.dottedName, DependencyType.formule]] } @@ -394,7 +405,8 @@ export function ruleDepsOfNode( } function ruleDepsOfRuleNode( - rule: ASTTypes.RuleNode + rule: ASTTypes.RuleNode, + parentRuleName: Names | '' ): RuleDependencies { const subNodes = [ rule.formule, @@ -402,15 +414,16 @@ function ruleDepsOfRuleNode( rule['non applicable si'] ].filter(x => x !== undefined) as Array const subNodesDeps = subNodes - .map(x => ruleDepsOfNode(rule.dottedName, x)) + .map(subNode => ruleDepsOfNode(rule.dottedName, subNode)) .flat(1) const isDisabledByDependencies: RuleDependencies = rule.isDisabledBy.map( x => [x.dottedName, DependencyType.isDisabledBy] ) - const replacedByDependencies: RuleDependencies = rule.replacedBy.map( - x => [x.referenceNode.dottedName, DependencyType.replacedBy] - ) + const replacedByDependencies: RuleDependencies = getApplicableReplacedBy( + parentRuleName, + rule.replacedBy + ).map(x => [x.referenceNode.dottedName, DependencyType.replacedBy]) return [subNodesDeps, isDisabledByDependencies, replacedByDependencies].flat( 1 ) @@ -433,6 +446,7 @@ export function buildRulesDependencies( ([dottedName, ruleNode]: [Names, ASTTypes.RuleNode]): [ Names, RuleDependencies - ] => [dottedName, ruleDepsOfRuleNode(ruleNode)] + // [XXX] Check how the root nodes are contextRuleName'd: + ] => [dottedName, ruleDepsOfRuleNode(ruleNode, '')] ) } diff --git a/publicodes/source/parseReference.js b/publicodes/source/parseReference.js index 10022bd22..a83c1fff4 100644 --- a/publicodes/source/parseReference.js +++ b/publicodes/source/parseReference.js @@ -30,7 +30,6 @@ export const getApplicableReplacedBy = (contextRuleName, replacedBy) => !blackListedNames || blackListedNames.every(name => !contextRuleName.startsWith(name)) ) - // ⚠️ this behavior is referenced in `cyclesLib/graph.ts` .filter(({ referenceNode }) => contextRuleName !== referenceNode.dottedName) /** diff --git a/publicodes/test/cycles.test.js b/publicodes/test/cycles.test.js index bc6d5d9f1..79c885fe1 100644 --- a/publicodes/test/cycles.test.js +++ b/publicodes/test/cycles.test.js @@ -1,12 +1,8 @@ +import yaml from 'yaml' import { expect } from 'chai' import dedent from 'dedent-js' -import graphlib from '@dagrejs/graphlib' -import { DependencyType } from '../source/cyclesLib/rulesDependencies' -import { - cyclicDependencies, - flattenOneLevelRemplaceLoops, - GraphError -} from '../source/cyclesLib/graph' +import { cyclicDependencies, GraphError } from '../source/cyclesLib/graph' +import Engine from '../source/index' describe('Naive dependencies builder', () => { it('catches double-dependecies', () => { @@ -21,75 +17,6 @@ describe('Naive dependencies builder', () => { }) }) -describe('ROLL flatten-o-tron 2500 ™', () => { - it('should replace 2 ROLL nodes with 4 nodes without loop', () => { - const g = new graphlib.Graph() - - g.setEdge('b', 'c', { type: DependencyType.formule }) - g.setEdge('c', 'b', { type: DependencyType.replacedBy }) - - const flattenedGraph = flattenOneLevelRemplaceLoops(g) - - expect(flattenedGraph.nodes()).to.deep.equal([ - 'b [depType: 0]', - 'c [depType: 0]', - 'c [depType: 1]', - 'b [depType: 1]' - ]) - expect(flattenedGraph.edges()).to.deep.equal([ - { v: 'b [depType: 0]', w: 'c [depType: 0]' }, - { v: 'c [depType: 1]', w: 'b [depType: 1]' } - ]) - }) - - it('should replace 2 ROLL nodes in context of a larger graph', () => { - const g = new graphlib.Graph() - - g.setEdge('a', 'b', { type: DependencyType.formule }) - g.setEdge('a', 'c', { type: DependencyType.formule }) - g.setEdge('b', 'c', { type: DependencyType.formule }) - g.setEdge('c', 'b', { type: DependencyType.replacedBy }) - g.setEdge('b', 'd', { type: DependencyType.formule }) - g.setEdge('c', 'd', { type: DependencyType.formule }) - - const flattenedGraph = flattenOneLevelRemplaceLoops(g) - - expect(flattenedGraph.nodes()).to.deep.equal([ - 'a', - 'b [depType: 1]', - 'b [depType: 0]', - 'c [depType: 1]', - 'c [depType: 0]', - 'd' - ]) - expect(flattenedGraph.edges()).to.have.deep.members([ - { v: 'a', w: 'b [depType: 0]' }, - { v: 'a', w: 'b [depType: 1]' }, - { v: 'a', w: 'c [depType: 0]' }, - { v: 'a', w: 'c [depType: 1]' }, - { v: 'b [depType: 0]', w: 'c [depType: 0]' }, - { v: 'c [depType: 1]', w: 'b [depType: 1]' }, - { v: 'b [depType: 0]', w: 'd' }, - { v: 'b [depType: 1]', w: 'd' }, - { v: 'c [depType: 0]', w: 'd' }, - { v: 'c [depType: 1]', w: 'd' } - ]) - }) - - it('should not replace any nodes in a 2-level formule + remplace loop', () => { - const g = new graphlib.Graph() - - g.setEdge('a', 'b', { type: DependencyType.formule }) - g.setEdge('b', 'c', { type: DependencyType.formule }) - g.setEdge('c', 'a', { type: DependencyType.replacedBy }) - - const flattenedGraph = flattenOneLevelRemplaceLoops(g) - - expect(flattenedGraph.nodes()).to.deep.equal(['a', 'b', 'c']) - expect(flattenedGraph.edges()).to.deep.equal(g.edges()) - }) -}) - describe('Cyclic dependencies detectron 3000 ™', () => { it('should detect the trivial formule cycle', () => { const rules = dedent`