Skip to content

Commit

Permalink
NEW warn when layout options (approximately) do not change branch len…
Browse files Browse the repository at this point in the history 
…gths (#456)

* NEW warn when ultrametric layout (approximately) does not change trees

* MAINT add comments to check lengths change

* MAINT remove old code comments

* DOC fix checker doc

* ENH use relative tolerance

* ENH move toast msg to function

* STY make style

* DOC documentation and message improvement

Co-authored-by: Marcus Fedarko <[email protected]>

* ENH extract update anyDifferent

* ENH modify logic surroudning raising warning

* Apply suggestions from code review

Co-authored-by: Marcus Fedarko <[email protected]>

* Use division for branch length comparison again

* Update empress/support_files/js/layouts-util.js

Co-authored-by: Marcus Fedarko <[email protected]>
  • Loading branch information
@gwarmstrong @fedarko
gwarmstrong and fedarko authored Jan 29, 2021
1 parent 65b3f01 commit ed581ad
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 6 deletions.
12 changes: 9 additions & 3 deletions empress/support_files/js/empress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,9 @@ define([
var data, i;
// set up length getter
var branchMethod = this.branchMethod;
var checkLengthsChange = LayoutsUtil.shouldCheckBranchLengthsChanged(
branchMethod
);
var lengthGetter = LayoutsUtil.getLengthMethod(
branchMethod,
this._tree
Expand All @@ -392,7 +395,8 @@ define([
// what lengths it should lay out.
this.leafSorting,
undefined,
lengthGetter
lengthGetter,
checkLengthsChange
);
this._yrscf = data.yScalingFactor;
for (i = 1; i <= this._tree.size; i++) {
Expand All @@ -414,7 +418,8 @@ define([
4020,
this.leafSorting,
undefined,
lengthGetter
lengthGetter,
checkLengthsChange
);
for (i = 1; i <= this._tree.size; i++) {
// remove old layout information
Expand All @@ -439,7 +444,8 @@ define([
4020,
4020,
undefined,
lengthGetter
lengthGetter,
checkLengthsChange
);
for (i = 1; i <= this._tree.size; i++) {
// remove old layout information
Expand Down
88 changes: 85 additions & 3 deletions empress/support_files/js/layouts-util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,40 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
return lengthGetter;
}

/**
* Determines whether a given method should check if branch lengths were changed.
*
* @param {String} methodName Method for determing branch lengths.
* One of ("ultrametric", "ignore", "normal").
* @returns {Boolean} Indicates whether the check should be performed.
* @throws {Error} If methodName is invalid.
*/
function shouldCheckBranchLengthsChanged(methodName) {
var methods = {
ultrametric: true,
ignore: true,
normal: false,
};
if (methodName in methods) {
return methods[methodName];
} else {
throw "Invalid method: '" + methodName + "'.";
}
}

var NO_LENGTHS_CHANGED_MSG =
"It doesn't look like any branch lengths were changed " +
"by the current method of modifying branch lengths.";
var NO_LENGTHS_CHANGED_DURATION = 3000;
var TOL = 0.000001;

/**
* Raises a toast message telling the user that no branch lengths changed.
*/
function noLengthsChangedMsg() {
util.toastMsg(NO_LENGTHS_CHANGED_MSG, NO_LENGTHS_CHANGED_DURATION);
}

/**
* Computes the "scale factor" for the circular / unrooted layouts.
*
Expand Down Expand Up @@ -266,6 +300,9 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
* that corresponds to the index of a node in
* tree. Returns the length of the node at that
* index. Defaults to 'normal' method.
* @param {Boolean} checkLengthsChange If true, then a warning will be raised
* if no branch lengths in the tree differ
* from the value determined by lengthGetter.
* @return {Object} Object with the following properties:
* -xCoords
* -yCoords
Expand All @@ -282,7 +319,8 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
height,
leafSorting,
normalize = true,
lengthGetter = null
lengthGetter = null,
checkLengthsChange = false
) {
var maxWidth = 0;
var maxHeight = 0;
Expand Down Expand Up @@ -322,6 +360,7 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
}
}

var anyDifferent = false;
// iterates in preorder
var parent;
for (i = 2; i <= tree.size; i++) {
Expand All @@ -334,6 +373,12 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
if (maxWidth < xCoord[node]) {
maxWidth = xCoord[node];
}
if (checkLengthsChange && !anyDifferent) {
anyDifferent = isTransformedLenDifferent(nodeLen, tree, prepos);
}
}
if (checkLengthsChange && !anyDifferent) {
noLengthsChangedMsg();
}

// We don't check if max_width == 0 here, because we check when
Expand Down Expand Up @@ -473,6 +518,9 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
* that corresponds to the index of a node in
* tree. Returns the length of the node at that
* index. Defaults to 'normal' method.
* @param {Boolean} checkLengthsChange If true, then a warning will be raised
* if no branch lengths in the tree differ
* from the value determined by lengthGetter.
* @return {Object} Object with the following properties:
* -x0, y0 ("starting point" x and y)
* -x1, y1 ("ending point" x and y)
Expand All @@ -492,7 +540,8 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
height,
leafSorting,
normalize = true,
lengthGetter = null
lengthGetter = null,
checkLengthsChange = false
) {
// Set up arrays we're going to store the results in
var x0 = new Array(tree.size + 1).fill(0);
Expand Down Expand Up @@ -556,6 +605,7 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
}
}

var anyDifferent = false;
// Iterate over the tree in preorder, assigning radii
// (The "i = 2" skips the root of the tree; its radius is implicitly 0)
for (i = 2; i <= tree.size; i++) {
Expand All @@ -568,6 +618,12 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {

var nodeLen = lengthGetter(prepos);
radius[node] = radius[parent] + nodeLen;
if (checkLengthsChange && !anyDifferent) {
anyDifferent = isTransformedLenDifferent(nodeLen, tree, prepos);
}
}
if (checkLengthsChange && !anyDifferent) {
noLengthsChangedMsg();
}

// Now that we have the polar coordinates of the nodes, convert them to
Expand Down Expand Up @@ -691,6 +747,20 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
};
}

/**
* Returns true if a given node (a.k.a. branch) in the tree has a
* transformed length differing from its original length.
*
* @param {Float} branchLen Transformed length of the branch.
* @param {BPTree} tree That has (potentially) been transformed.
* @param {Number} n Node index in tree.
* @returns {Boolean} Indicates if this branch's transformed length is
* different from its original length.
*/
function isTransformedLenDifferent(branchLen, tree, n) {
return Math.abs(tree.length(n) / branchLen - 1) > TOL;
}

/**
* Unrooted layout.
*
Expand All @@ -709,6 +779,9 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
* that corresponds to the index of a node in
* tree. Returns the length of the node at that
* index. Defaults to 'normal' method.
* @param {Boolean} checkLengthsChange If true, then a warning will be raised
* if no branch lengths in the tree differ
* from the value determined by lengthGetter.
* @return {Object} Object with the following properties:
* -xCoords
* -yCoords
Expand All @@ -720,7 +793,8 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
width,
height,
normalize = true,
lengthGetter = null
lengthGetter = null,
checkLengthsChange = false
) {
var da = (2 * Math.PI) / tree.numleaves();
var x1Arr = new Array(tree.size + 1);
Expand All @@ -747,6 +821,7 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
var maxY = y2Arr[tree.size],
minY = y2Arr[tree.size];

var anyDifferent = false;
// reverse postorder
for (var node = tree.size - 1; node > 0; node--) {
var parent = tree.postorder(
Expand Down Expand Up @@ -776,6 +851,12 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
minX = Math.min(minX, x2Arr[node]);
maxY = Math.max(maxY, y2Arr[node]);
minY = Math.min(minY, y2Arr[node]);
if (checkLengthsChange && !anyDifferent) {
anyDifferent = isTransformedLenDifferent(nodeLen, tree, n);
}
}
if (checkLengthsChange && !anyDifferent) {
noLengthsChangedMsg();
}
if (normalize) {
var scaleFactor = computeScaleFactor(
Expand All @@ -802,6 +883,7 @@ define(["underscore", "VectorOps", "util"], function (_, VectorOps, util) {
getLengthMethod: getLengthMethod,
getPostOrderNodes: getPostOrderNodes,
getUltrametricLengths: getUltrametricLengths,
shouldCheckBranchLengthsChanged: shouldCheckBranchLengthsChanged,
computeScaleFactor: computeScaleFactor,
rectangularLayout: rectangularLayout,
circularLayout: circularLayout,
Expand Down

0 comments on commit ed581ad

Please sign in to comment.