Skip to content

Fix exponential ancestor/descendant traversal caused by pedigree collapse #110

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.

Sign up for GitHub

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

Open
robekl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix exponential ancestor/descendant traversal caused by pedigree collapse #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 21 additions & 9 deletions src/ancestor-chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import {
Fam,
Indi,
TreeNode,
} from './api';
import { ChartUtil, getChartInfo } from './chart-util';
import { HierarchyNode, stratify } from 'd3-hierarchy';
import { IdGenerator } from './id-generator';
} from "./api";
import { ChartUtil, getChartInfo } from "./chart-util";
import { HierarchyNode, stratify } from "d3-hierarchy";
import { IdGenerator } from "./id-generator";

export function getAncestorsTree(options: ChartOptions) {
const ancestorChartOptions = { ...options };
Expand Down Expand Up @@ -45,9 +45,10 @@ export function getAncestorsTree(options: ChartOptions) {
}

/** Renders an ancestors chart. */
export class AncestorChart<IndiT extends Indi, FamT extends Fam>
implements Chart
{
export class AncestorChart<
IndiT extends Indi,
FamT extends Fam,
> implements Chart {
readonly util: ChartUtil;

constructor(readonly options: ChartOptions) {
Expand Down Expand Up @@ -85,12 +86,23 @@ export class AncestorChart<IndiT extends Indi, FamT extends Fam>
});
}

// Track real family IDs already added to the hierarchy. Without this,
// pedigree collapse (endogamy) causes the same ancestral family to be
// visited exponentially — each duplicate visit re-queues its parents,
// producing O(2^n) iterations and a tree too large to render.
const addedFamilies = new Set<string>();

while (stack.length) {
const entry = stack.pop()!;
const fam = this.options.data.getFam(entry.family!.id);
if (!fam) {
continue;
}
// If this family has already been added via another ancestral path,
// show a PLUS expander rather than re-traversing its parents.
const alreadyAdded = addedFamilies.has(entry.family!.id);
addedFamilies.add(entry.family!.id);

const [father, mother] =
entry.family!.id === this.options.startFam &&
this.options.swapStartSpouses
Expand All @@ -104,7 +116,7 @@ export class AncestorChart<IndiT extends Indi, FamT extends Fam>
const indi = this.options.data.getIndi(mother)!;
const famc = indi.getFamilyAsChild();
if (famc) {
if (this.options.collapsedSpouse?.has(entry.id)) {
if (alreadyAdded || this.options.collapsedSpouse?.has(entry.id)) {
entry.spouse.expander = ExpanderState.PLUS;
} else {
const id = idGenerator.getId(famc);
Expand All @@ -123,7 +135,7 @@ export class AncestorChart<IndiT extends Indi, FamT extends Fam>
const indi = this.options.data.getIndi(father)!;
const famc = indi.getFamilyAsChild();
if (famc) {
if (this.options.collapsedIndi?.has(entry.id)) {
if (alreadyAdded || this.options.collapsedIndi?.has(entry.id)) {
entry.indi.expander = ExpanderState.PLUS;
} else {
const id = idGenerator.getId(famc);
Expand Down
31 changes: 22 additions & 9 deletions src/descendant-chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HierarchyNode, HierarchyPointNode, stratify } from 'd3-hierarchy';
import { HierarchyNode, HierarchyPointNode, stratify } from "d3-hierarchy";
import {
Chart,
ChartInfo,
Expand All @@ -7,11 +7,11 @@ import {
Fam,
Indi,
TreeNode,
} from './api';
import { ChartUtil, getChartInfo, LayoutOptions } from './chart-util';
import { IdGenerator } from './id-generator';
} from "./api";
import { ChartUtil, getChartInfo, LayoutOptions } from "./chart-util";
import { IdGenerator } from "./id-generator";

export const DUMMY_ROOT_NODE_ID = 'DUMMY_ROOT_NODE';
export const DUMMY_ROOT_NODE_ID = "DUMMY_ROOT_NODE";

export function layOutDescendants(
options: ChartOptions,
Expand Down Expand Up @@ -57,9 +57,10 @@ function getSpouse(indiId: string, fam: Fam): string | null {
}

/** Renders a descendants chart. */
export class DescendantChart<IndiT extends Indi, FamT extends Fam>
implements Chart
{
export class DescendantChart<
IndiT extends Indi,
FamT extends Fam,
> implements Chart {
readonly util: ChartUtil;

constructor(readonly options: ChartOptions) {
Expand Down Expand Up @@ -144,6 +145,12 @@ export class DescendantChart<IndiT extends Indi, FamT extends Fam>

parents.push(...nodes);

// Track real family IDs already added to prevent exponential blowup from
// pedigree collapse (consanguineous marriages) in descendant trees.
const addedFamilies = new Set<string>(
nodes.filter((n) => n.family).map((n) => n.family!.id),
);

const stack: TreeNode[] = [];
nodes.forEach((node) => {
if (node.family) {
Expand All @@ -166,8 +173,14 @@ export class DescendantChart<IndiT extends Indi, FamT extends Fam>
childNodes.forEach((node) => {
node.parentId = entry.id;
if (node.family) {
const alreadyAdded = addedFamilies.has(node.family.id);
node.id = `${idGenerator.getId(node.family.id)}`;
stack.push(node);
if (alreadyAdded) {
node.family.expander = ExpanderState.PLUS;
} else {
addedFamilies.add(node.family.id);
stack.push(node);
}
}
});
parents.push(...childNodes);
Expand Down
94 changes: 70 additions & 24 deletions tests/ancestor-chart.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
/// <reference path="./jsdom-global.d.ts" />
import * as jsdomGlobal from 'jsdom-global';
import * as jsdomGlobal from "jsdom-global";

import {AncestorChart} from '../src/ancestor-chart';
import {JsonDataProvider, JsonGedcomData} from '../src/data';
import { AncestorChart } from "../src/ancestor-chart";
import { JsonDataProvider, JsonGedcomData } from "../src/data";

import {FakeRenderer} from './fake_renderer';
import { FakeRenderer } from "./fake_renderer";

// Initialize DOM.
(jsdomGlobal as any)(); // tslint:disable-line
(jsdomGlobal as any)(); // tslint:disable-line

describe('Ancestor chart', () => {
describe("Ancestor chart", () => {
beforeEach(() => {
document.body.innerHTML = '<svg></svg>';
document.body.innerHTML = "<svg></svg>";
});

it('should work for a single person', () => {
const json: JsonGedcomData = {fams: [], indis: [{id: 'I1'}]};
it("should work for a single person", () => {
const json: JsonGedcomData = { fams: [], indis: [{ id: "I1" }] };
const data = new JsonDataProvider(json);
const chart = new AncestorChart({
data,
startIndi: 'I1',
startIndi: "I1",
renderer: new FakeRenderer(),
svgSelector: 'svg',
svgSelector: "svg",
});
chart.render();
expect(document.querySelectorAll('g.node').length).toEqual(1);
expect(document.querySelectorAll("g.node").length).toEqual(1);
});

it('should work with a common ancestor of two spouses', () => {
it("should work with a common ancestor of two spouses", () => {
// I5+I6
// F3
// |
Expand All @@ -40,26 +40,72 @@ describe('Ancestor chart', () => {
// F1
const json: JsonGedcomData = {
fams: [
{id: 'F1', husb: 'I1', wife: 'I2'},
{id: 'F2', husb: 'I3', wife: 'I4', children: ['I1', 'I2']},
{id: 'F3', husb: 'I5', wife: 'I6', children: ['I3']},
{ id: "F1", husb: "I1", wife: "I2" },
{ id: "F2", husb: "I3", wife: "I4", children: ["I1", "I2"] },
{ id: "F3", husb: "I5", wife: "I6", children: ["I3"] },
],
indis: [
{id: 'I1', fams: ['F1'], famc: 'F2'},
{id: 'I2', fams: ['F1'], famc: 'F2'},
{id: 'I3', fams: ['F2'], famc: 'F3'},
{id: 'I4', fams: ['F2']},
{id: 'I5', fams: ['F3']},
{id: 'I6', fams: ['F3']},
{ id: "I1", fams: ["F1"], famc: "F2" },
{ id: "I2", fams: ["F1"], famc: "F2" },
{ id: "I3", fams: ["F2"], famc: "F3" },
{ id: "I4", fams: ["F2"] },
{ id: "I5", fams: ["F3"] },
{ id: "I6", fams: ["F3"] },
],
};
const data = new JsonDataProvider(json);
const chart = new AncestorChart({
data,
startFam: 'F1',
startFam: "F1",
renderer: new FakeRenderer(),
svgSelector: 'svg',
svgSelector: "svg",
});
chart.render();
});

it("should not expand the same ancestor family more than once (pedigree collapse)", () => {
// Cousins I1 and I2 marry. Their shared grandparents (I5+I6 in F3) appear
// in both the I1 and I2 ancestry paths. Without cycle detection the chart
// traverses F3 exponentially, causing the browser to hang.
//
// I5+I6
// F3
// / \
// I3+? I4+?
// F1 F2
// | |
// I1 + I2 <- cousins who married
// F4
const json: JsonGedcomData = {
fams: [
{ id: "F1", husb: "I3", children: ["I1"] },
{ id: "F2", wife: "I4", children: ["I2"] },
{ id: "F3", husb: "I5", wife: "I6", children: ["I3", "I4"] },
{ id: "F4", husb: "I1", wife: "I2" },
],
indis: [
{ id: "I1", fams: ["F4"], famc: "F1" },
{ id: "I2", fams: ["F4"], famc: "F2" },
{ id: "I3", fams: ["F1"], famc: "F3" },
{ id: "I4", fams: ["F2"], famc: "F3" },
{ id: "I5", fams: ["F3"] },
{ id: "I6", fams: ["F3"] },
],
};
const data = new JsonDataProvider(json);
const chart = new AncestorChart({
data,
startFam: "F4",
renderer: new FakeRenderer(),
svgSelector: "svg",
});
// Should complete without hanging. F3 must appear exactly once in the
// rendered output; the second path to F3 (via I4) is collapsed to a PLUS
// expander. Without cycle detection this would grow exponentially.
chart.render();
const nodes = document.querySelectorAll("g.node");
// The rendered set is bounded: start family + each unique ancestral family
// visited once. Verify it is well below an exponential worst-case.
expect(nodes.length).toBeLessThan(20);
});
});
89 changes: 67 additions & 22 deletions tests/descendant-chart.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
/// <reference path="./jsdom-global.d.ts" />
import * as jsdomGlobal from 'jsdom-global';
import * as jsdomGlobal from "jsdom-global";

import {JsonDataProvider, JsonGedcomData} from '../src/data';
import {DescendantChart} from '../src/descendant-chart';
import { JsonDataProvider, JsonGedcomData } from "../src/data";
import { DescendantChart } from "../src/descendant-chart";

import {FakeRenderer} from './fake_renderer';
import { FakeRenderer } from "./fake_renderer";

// Initialize DOM.
(jsdomGlobal as any)(); // tslint:disable-line
(jsdomGlobal as any)(); // tslint:disable-line

describe('Descendant chart', () => {
describe("Descendant chart", () => {
beforeEach(() => {
document.body.innerHTML = '<svg></svg>';
document.body.innerHTML = "<svg></svg>";
});

it('should work for a single person', () => {
const json: JsonGedcomData = {fams: [], indis: [{id: 'I1'}]};
it("should work for a single person", () => {
const json: JsonGedcomData = { fams: [], indis: [{ id: "I1" }] };
const data = new JsonDataProvider(json);
const chart = new DescendantChart({
data,
startIndi: 'I1',
startIndi: "I1",
renderer: new FakeRenderer(),
svgSelector: 'svg',
svgSelector: "svg",
});
chart.render();
expect(document.querySelectorAll('g.node').length).toEqual(1);
expect(document.querySelectorAll("g.node").length).toEqual(1);
});

it('should work with a common descendant of two siblings', () => {
it("should work with a common descendant of two siblings", () => {
// I1+I2
// F1
// |
Expand All @@ -39,24 +39,69 @@ describe('Descendant chart', () => {
// I5
const json: JsonGedcomData = {
fams: [
{id: 'F1', husb: 'I1', wife: 'I2', children: ['I3', 'I4']},
{id: 'F2', husb: 'I3', wife: 'I4', children: ['I5']},
{ id: "F1", husb: "I1", wife: "I2", children: ["I3", "I4"] },
{ id: "F2", husb: "I3", wife: "I4", children: ["I5"] },
],
indis: [
{id: 'I1', fams: ['F1']},
{id: 'I2', fams: ['F1']},
{id: 'I3', fams: ['F2'], famc: 'F1'},
{id: 'I4', fams: ['F2'], famc: 'F1'},
{id: 'I5', famc: 'F2'},
{ id: "I1", fams: ["F1"] },
{ id: "I2", fams: ["F1"] },
{ id: "I3", fams: ["F2"], famc: "F1" },
{ id: "I4", fams: ["F2"], famc: "F1" },
{ id: "I5", famc: "F2" },
],
};
const data = new JsonDataProvider(json);
const chart = new DescendantChart({
data,
startFam: 'F1',
startFam: "F1",
renderer: new FakeRenderer(),
svgSelector: 'svg',
svgSelector: "svg",
});
chart.render();
});

it("should not expand the same descendant family more than once (pedigree collapse)", () => {
// Two siblings (I3 and I4) who are children of F1 each marry and have
// children who intermarry (I5 and I6 both marry into F4, and F4's child
// I7 marries back into the line via F5). Without cycle detection the
// descendant chart revisits F4 from multiple paths, growing exponentially.
//
// F1 (I1+I2)
// ├─ I3 ─ F2
// │ └─ I5 ─ F4
// └─ I4 ─ F3 └─ I7
// └─ I6 ─ F4 (same family, second path)
const json: JsonGedcomData = {
fams: [
{ id: "F1", husb: "I1", wife: "I2", children: ["I3", "I4"] },
{ id: "F2", husb: "I3", children: ["I5"] },
{ id: "F3", wife: "I4", children: ["I6"] },
{ id: "F4", husb: "I5", wife: "I6", children: ["I7"] },
],
indis: [
{ id: "I1", fams: ["F1"] },
{ id: "I2", fams: ["F1"] },
{ id: "I3", fams: ["F2"], famc: "F1" },
{ id: "I4", fams: ["F3"], famc: "F1" },
{ id: "I5", fams: ["F4"], famc: "F2" },
{ id: "I6", fams: ["F4"], famc: "F3" },
{ id: "I7", famc: "F4" },
],
};
const data = new JsonDataProvider(json);
const chart = new DescendantChart({
data,
startFam: "F1",
renderer: new FakeRenderer(),
svgSelector: "svg",
});
// Should complete without hanging. F4 must appear exactly once in the
// rendered output; the second path to F4 (via I6) is collapsed to a PLUS
// expander. Without cycle detection this would grow exponentially.
chart.render();
const nodes = document.querySelectorAll("g.node");
// The rendered set is bounded: start family + each unique descendant family
// visited once. Verify it is well below an exponential worst-case.
expect(nodes.length).toBeLessThan(20);
});
});
Loading