search

knncolle / BiocNeighbors

Clone of the Bioconductor repository for the BiocNeighbors package.
https://bioconductor.org/packages/devel/bioc/html/BiocNeighbors.html
6 stars 11 forks source link

RcppHNSW 0.6.0/hnswlib 0.8.0 may require defining HNSWLIB_ERR_OVERRIDE #23

Open jlmelville opened 11 months ago

jlmelville commented 11 months ago

hnswlib 0.8.0 was recently released, and adds calls to std::cerr, under some conditions. As part of preparing RcppHNSW for a new CRAN release, BiocNeighbors is causing a reverse dependency check failures due to triggering a CRAN check WARNING.

I am not sure how much CRAN cares about Bioconductor reverse dependency changes (or if it's a problem for this package to call std::cerr), but to fix this, you could create a header file like:

#include <Rcpp.h>

#define HNSWLIB_ERR_OVERRIDE Rcpp::Rcerr

#include "hnswlib.h"

and include that file wherever you are including hnswlib.h currently. I seem to recall there was something similar with hnswlib and using a #define to opt out of the non-standard optimizations for the distance calculations.

See https://github.com/nmslib/hnswlib/pull/508 for more on the hnswlib side.

LTLA commented 10 months ago

Thanks @jlmelville; sorry for the late reply. I've added the suggested #define to hnsw.h, which should cover all of the within-package usages of hnswlib.h (see 8abf622aa7dbf780702b2fe5e76312789be6cf94). This has been pushed to BioC-devel, and if it builds fine there (and if I remember), I'll push it to BioC-release.

jlmelville commented 10 months ago

Thanks @LTLA I assume that I can just look at https://bioconductor.org/packages/release/bioc/html/BiocNeighbors.html and when it reaches 1.21.2 or above it's safe to retry the reverse dependency check?

LTLA commented 10 months ago

Oops, forgot about this. Just pushed the same change to the release stream; in a few days, it should pop out as 1.20.2.