Skip to content

Commit

Permalink
Merged release/0.1.10.2 into master
Browse files Browse the repository at this point in the history
  • Loading branch information
elbamos committed Nov 27, 2016
2 parents 3052eb4 + 175bc8a commit 7c920fa
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 59 deletions.
16 changes: 10 additions & 6 deletions cran-comments.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
## Submission

This is a fix to an issue that appeared in 0.1.10 after submission. In particular, if largeVis was compiled to use 32-bit Armadillo words, with certain compilers including the most recent Apple compiler, the parameter specifying the number of training batches was not being passed from R to C++ properly. This caused the tests to time-out and checks to fail on those systems. (At the same time, this release fixes a reported bug where the neighbor search could fail with sparse matrices under certain conditions.)
This is a resubmisison of a minor version update intended as a hotfix. The response to the last submission was to inquire about test errors with version 0.1.10 and builds of R devel 3.4.

I’ve tested the current submission against R-devel 3.3.2 on linux https://travis-ci.org/elbamos/largeVis/jobs/178220156, R-devel using win builder, and 3.4 on OS X on my own machine. I’ve tested with valgrind on my own machine using both 32-bit and 64-bit builds.

I believe the error that was showing in the 0.1.10-r-3.4 log is related to the bug fix that is the reason for this minor version update. The error was occuring on an example that is very similar to an existing test. I have moved the example to a test intended to replicate it exactly and encased the example version in \dontrun{}.

## Test environments
* local OS X install, R 3.3.1
* OS X (with Valgrind), R 3.3.1
* ubuntu 12.04 (on travis-ci), R 3.3.1
* ubuntu 14.04 (on travis-ci), R 3.3.1 and R-devel
* local OS X install, R 3.3.2
* OS X (with Valgrind), R 3.3.2
* ubuntu 12.04 (on travis-ci), R 3.3.2
* ubuntu 14.04 (on travis-ci), R 3.3.2 and R-devel
* Solaris 11 x86 (via Virtual Box), R 3.3.1
* OS X (on travis-ci), R-devel
* win-builder (devel and release)
Expand All @@ -19,7 +23,7 @@ This is a fix to an issue that appeared in 0.1.10 after submission. In particula

## Note regarding test errors

The submission dialog asks me to confirm that CI errors have been fixed. This submission is intended to fix those errors, except that one of the errors concerns Windows with the old release of R. The log shows that the compiler in this test is forcing the use of C++ standard 0X even though `Makevars` requires C++11. I believe requiring C++11 is permissible under the CRAN standards and this is specified in the DESCRIPTION file.
The submission dialog asks me to confirm that CI errors have been fixed. This submission is intended to fix those errors, except that one of the errors concerns Windows with the old release of R. The log for the old-Windows build shows that the compiler in this test is forcing the use of C++ standard 0X even though `Makevars` requires C++11. I believe requiring C++11 is permissible under the CRAN standards and this is specified in the DESCRIPTION file.

## Reverse dependencies

Expand Down
8 changes: 4 additions & 4 deletions inst/doc/largeVis.html

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions inst/doc/momentumandusedata.html

Large diffs are not rendered by default.

Binary file removed inst/testdata/jaindata.Rda
Binary file not shown.
25 changes: 12 additions & 13 deletions src/largeVis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,25 +228,23 @@ arma::mat sgd(arma::mat& coords,
const vertexidxtype N = coords.n_cols;
const edgeidxtype E = targets_i.n_elem;

if (D > 10) stop("Limit of 10 dimensions for low-dimensional space.");
Visualizer* v;
if (momentum.isNull()) v = new Visualizer( sources_j.memptr(),
targets_i.memptr(),
coords.memptr(),
D, N, E,
rho, n_samples,
M, alpha, gamma);
if (momentum.isNull()) v = new Visualizer(
sources_j.memptr(), targets_i.memptr(), coords.memptr(),
D, N, E,
rho, n_samples,
M, alpha, gamma);
else {
float moment = NumericVector(momentum)[0];
if (moment < 0) stop("Momentum cannot be negative.");
if (moment > 1) stop("Momentum canot be > 1.");
v = new MomentumVisualizer(sources_j.memptr(),
targets_i.memptr(),
coords.memptr(),
D, N, E,
rho, n_samples, moment,
M, alpha, gamma);
v = new MomentumVisualizer(
sources_j.memptr(), targets_i.memptr(), coords.memptr(),
D, N, E,
rho, n_samples, moment,
M, alpha, gamma);
}

distancetype* negweights = new distancetype[N];
for (vertexidxtype n = 0; n < N; ++n) negweights[n] = 0;
if (useDegree) {
Expand All @@ -261,6 +259,7 @@ arma::mat sgd(arma::mat& coords,
for (vertexidxtype n = 0; n < N; ++n) negweights[n] = pow(negweights[n], 0.75);
v -> initAlias(weights.memptr(), negweights, seed);
delete[] negweights;

const unsigned int batchSize = 8192;
const iterationtype barrier = (n_samples * .99 < n_samples - coords.n_cols) ? n_samples * .99 : n_samples - coords.n_cols;
#ifdef _OPENMP
Expand Down
31 changes: 0 additions & 31 deletions tests/testthat/test_g_dbandoptics.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,6 @@ test_that("dbscan works with largeVis objects", {
expect_lte(sum(cl$cluster != irisclustering$cluster), 1)
})

context("dbscan-jain")

test_that("dbscan matches dbscan on jain when the neighborhoods are complete", {
load(system.file(package = "largeVis", "testdata/jaindata.Rda"))
jainclusters <- lv_dbscan(edges = jaindata$edges, neighbors = jaindata$neighbors, eps = 2.5, minPts = 10, verbose = FALSE)
expect_equal(jainclusters$cluster, jaindata$dbclusters25$cluster)
})

context("optics-iris")

set.seed(1974)
Expand Down Expand Up @@ -98,29 +90,6 @@ test_that("optics works with largeVis objects", {
expect_equal(cl$coredist[!is.infinite(cl$coredist)], irisoptics$coredist[!is.infinite(irisoptics$coredist)])
})

context("optics-jain")

load(system.file(package = "largeVis", "testdata/jaindata.Rda"))
jainclusters <- lv_optics(edges = jaindata$edges,
neighbors = jaindata$neighbors,
eps = 2.5, minPts = 5, useQueue = FALSE,
verbose = FALSE)

test_that("optics matches optics core on jain when the neighborhoods are complete", {
expect_equal(is.infinite(jainclusters$coredist), is.infinite(jaindata$optics$coredist))
selections <- !is.infinite(jainclusters$coredist) & !is.infinite(jaindata$optics$coredist)
expect_equal(jainclusters$coredist[selections], jaindata$optics$coredist[selections])
})

test_that("optics matches dbscan on jain when the neighborhoods are complete", {
cl <- cutoptics(jainclusters)
expect_lte(sum(cl != (jaindata$dbclusters5$cluster + 1)), 2)
})

test_that("optics matches optics reachdist on jain when the neighborhoods are complete", {
expect_equal(is.infinite(jainclusters$reachdist), is.infinite(jaindata$optics$reachdist))
})

context("optics-elki")

test_that("optics output format is correct", {
Expand Down
7 changes: 6 additions & 1 deletion tests/testthat/test_h_hdbscan.R
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ neighbors <- randomProjectionTreeSearch(dat, K = K, threads = 2, verbose = FALS
edges <- buildEdgeMatrix(data = dat, neighbors = neighbors, verbose = FALSE)

test_that("as.dendrogram succeeds on iris4", {

hdobj <- hdbscan(edges, neighbors = neighbors, minPts = 10, K = 4, threads = 2, verbose = FALSE)
dend <- as_dendrogram_hdbscan(hdobj)
expect_true(length(dend[[1]]) == sum(hdobj$hierarchy$nodemembership == 1) + sum(hdobj$hierarchy$parent == 1) - 1 |
Expand All @@ -109,6 +108,12 @@ test_that("as.dendrogram succeeds on iris3", {
expect_equal(nobs(dend), ncol(dat))
} )

test_that("failing example doesn't fail", {
data(iris)
expect_silent(vis <- largeVis(t(iris[,1:4]), K = 20, sgd_batches = 1, threads = 2))
expect_silent(hdbscanobj <- hdbscan(vis, minPts = 10, K = 5, threads = 2))
})

context("gplot")

set.seed(1974)
Expand Down

0 comments on commit 7c920fa

Please sign in to comment.