diff --git a/.github/workflows/R_CMD_check_Hades.yaml b/.github/workflows/R_CMD_check_Hades.yaml index 94a9379b..04a216d4 100644 --- a/.github/workflows/R_CMD_check_Hades.yaml +++ b/.github/workflows/R_CMD_check_Hades.yaml @@ -20,12 +20,12 @@ jobs: fail-fast: false matrix: config: - - {os: windows-latest, r: 'release'} # Does not appear to have Java 32-bit, hence the --no-multiarch + - {os: windows-latest, r: 'release'} - {os: macOS-latest, r: 'release'} - {os: ubuntu-20.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"} -# - {os: ubuntu-20.04, r: 'devel', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"} env: + GITHUB_PAT: ${{ secrets.GH_TOKEN }} R_REMOTES_NO_ERRORS_FROM_WARNINGS: true RSPM: ${{ matrix.config.rspm }} CDM5_ORACLE_CDM_SCHEMA: ${{ secrets.CDM5_ORACLE_CDM_SCHEMA }} @@ -43,75 +43,51 @@ jobs: CDM5_SQL_SERVER_PASSWORD: ${{ secrets.CDM5_SQL_SERVER_PASSWORD }} CDM5_SQL_SERVER_SERVER: ${{ secrets.CDM5_SQL_SERVER_SERVER }} CDM5_SQL_SERVER_USER: ${{ secrets.CDM5_SQL_SERVER_USER }} + CDM5_REDSHIFT_CDM_SCHEMA: ${{ secrets.CDM5_REDSHIFT_CDM_SCHEMA }} + CDM5_REDSHIFT_OHDSI_SCHEMA: ${{ secrets.CDM5_REDSHIFT_OHDSI_SCHEMA }} + CDM5_REDSHIFT_PASSWORD: ${{ secrets.CDM5_REDSHIFT_PASSWORD }} + CDM5_REDSHIFT_SERVER: ${{ secrets.CDM5_REDSHIFT_SERVER }} + CDM5_REDSHIFT_USER: ${{ secrets.CDM5_REDSHIFT_USER }} + CDM5_SPARK_USER: ${{ secrets.CDM5_SPARK_USER }} + CDM5_SPARK_PASSWORD: ${{ secrets.CDM5_SPARK_PASSWORD }} + CDM5_SPARK_CONNECTION_STRING: ${{ secrets.CDM5_SPARK_CONNECTION_STRING }} + WEBAPI_TEST_WEBAPI_URL: ${{ secrets.WEBAPI_TEST_WEBAPI_URL }} + WEBAPI_TEST_SECURE_WEBAPI_URL: ${{ secrets.WEBAPI_TEST_SECURE_WEBAPI_URL }} + WEBAPI_TEST_ADMIN_USER_NAME: ${{ secrets.WEBAPI_TEST_ADMIN_USER_NAME }} + WEBAPI_TEST_ADMIN_USER_PASSWORD: ${{ secrets.WEBAPI_TEST_ADMIN_USER_PASSWORD }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - - uses: r-lib/actions/setup-r@v1 + - uses: r-lib/actions/setup-r@v2 with: r-version: ${{ matrix.config.r }} - - uses: r-lib/actions/setup-tinytex@v1 + - uses: r-lib/actions/setup-tinytex@v2 - - uses: r-lib/actions/setup-pandoc@v1 + - uses: r-lib/actions/setup-pandoc@v2 - - name: Query dependencies - run: | - install.packages('remotes') - saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) - writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version") - shell: Rscript {0} - - - name: Cache R packages - if: runner.os != 'Windows' - uses: actions/cache@v2 - with: - path: ${{ env.R_LIBS_USER }} - key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }} - restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1- - - - name: Install system dependencies + - name: Install system requirements if: runner.os == 'Linux' run: | + sudo apt-get install -y libssh-dev + Rscript -e 'install.packages("remotes")' while read -r cmd do eval sudo $cmd done < <(Rscript -e 'writeLines(remotes::system_requirements("ubuntu", "20.04"))') - - name: Install libssh - if: runner.os == 'Linux' - run: | - sudo apt-get install libssh-dev - - - name: Install dependencies - run: | - remotes::install_deps(dependencies = TRUE, INSTALL_opts=c("--no-multiarch")) - remotes::install_cran("rcmdcheck") - shell: Rscript {0} - - - name: Install covr - if: runner.os == 'macOS' - run: | - remotes::install_cran("covr") - shell: Rscript {0} - - - name: Remove check folder if exists - if: runner.os == 'macOS' - run: unlink("check", recursive = TRUE) - shell: Rscript {0} - - - name: Check - env: - _R_CHECK_CRAN_INCOMING_REMOTE_: false - run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran", "--no-multiarch"), error_on = "warning", check_dir = "check") - shell: Rscript {0} + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::rcmdcheck + needs: check - - name: Upload check results - if: failure() - uses: actions/upload-artifact@v2 + - uses: r-lib/actions/check-r-package@v2 with: - name: ${{ runner.os }}-r${{ matrix.config.r }}-results - path: check + args: 'c("--no-manual", "--as-cran")' + build_args: 'c("--compact-vignettes=both")' + error-on: '"warning"' + check-dir: '"check"' - name: Upload source package if: success() && runner.os == 'macOS' && github.event_name != 'pull_request' && github.ref == 'refs/heads/main' @@ -120,6 +96,12 @@ jobs: name: package_tarball path: check/*.tar.gz + - name: Install covr + if: runner.os == 'macOS' + run: | + install.packages("covr") + shell: Rscript {0} + - name: Test coverage if: runner.os == 'macOS' run: covr::codecov() @@ -137,7 +119,7 @@ jobs: steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: fetch-depth: 0 @@ -163,7 +145,7 @@ jobs: draft: false prerelease: false - - uses: r-lib/actions/setup-r@v1 + - uses: r-lib/actions/setup-r@v2 if: ${{ env.new_version != '' }} - name: Install drat @@ -192,3 +174,4 @@ jobs: if: ${{ env.new_version != '' }} run: | curl --data "build=true" -X POST https://registry.hub.docker.com/u/ohdsi/broadsea-methodslibrary/trigger/f0b51cec-4027-4781-9383-4b38b42dd4f5/ + diff --git a/Cyclops.Rproj b/Cyclops.Rproj index 235f524b..b68581e0 100644 --- a/Cyclops.Rproj +++ b/Cyclops.Rproj @@ -1,7 +1,7 @@ Version: 1.0 -RestoreWorkspace: Default -SaveWorkspace: Default +RestoreWorkspace: No +SaveWorkspace: No AlwaysSaveHistory: Default EnableCodeIndexing: Yes diff --git a/DESCRIPTION b/DESCRIPTION index fa04e463..c6352b83 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: Cyclops Type: Package Title: Cyclic Coordinate Descent for Logistic, Poisson and Survival Analysis -Version: 3.3.1.99 +Version: 3.3.1.999 Authors@R: c( person("Marc A.", "Suchard", email = "msuchard@ucla.edu", role = c("aut","cre")), person("Martijn J.", "Schuemie", role = "aut"), @@ -30,7 +30,7 @@ Biarch: true URL: https://github.com/ohdsi/cyclops BugReports: https://github.com/ohdsi/cyclops/issues Depends: - R (>= 3.1.0) + R (>= 3.5.0) Imports: rlang, Matrix, @@ -41,7 +41,6 @@ Imports: survival, bit64 LinkingTo: Rcpp, - BH (>= 1.51.0), RcppEigen (>= 0.3.2), RcppParallel Suggests: diff --git a/NEWS.md b/NEWS.md index 943d8096..0c8f5c71 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,19 +1,22 @@ -develop +Cyclops v3.4.0 ============== Changes: -1. improvements on adaptive likelihood profiling -2. add `auto` option to `cvRepetitions` -3. bumped explicit C++11 requirement up to R v4.1 +1. remove dependence on `BH` +2. improvements on adaptive likelihood profiling +3. add `auto` option to `cvRepetitions` +4. bumped explicit C++11 requirement up to R v4.1 +5. removed deprecated use of `dbplyr:::$.tbl_lazy` + a. breaking change in `dbplyr v2.4.0` Cyclops v3.3.1 ============== Changes: -1. fix uninitialized value in detected in computeAsymptoticPrecisionMatrix(); value was priorType -2. fix memory leak caused by call to ::Rf_error() +1. fix uninitialized value in detected in `computeAsymptoticPrecisionMatrix()`; value was priorType +2. fix memory leak caused by call to `::Rf_error()` 3. fix line-endings on Makevar on windows Cyclops v3.3.0 diff --git a/R/DataManagement.R b/R/DataManagement.R index d916010b..16e6eae6 100644 --- a/R/DataManagement.R +++ b/R/DataManagement.R @@ -634,8 +634,8 @@ reduce <- function(object, covariates, groupBy, power = 1) { #' \code{appendSqlCyclopsData} appends data to an OHDSI data object. #' #' @details Append data using two tables. The outcomes table is dense and contains ... The covariates table is sparse and contains ... -#' All entries in the outcome table must be sorted in increasing order by {oStratumId, oRowId}. All entries in the covariate table -#' must be sorted in increasing order by {cRowId}. Each cRowId value must match exactly one oRowId value. +#' All entries in the outcome table must be sorted in increasing order by (oStratumId, oRowId). All entries in the covariate table +#' must be sorted in increasing order by (cRowId). Each cRowId value must match exactly one oRowId value. #' #' @param object OHDSI Cyclops data object to append entries #' @param oStratumId Integer vector (optional): non-unique stratum identifier for each row in outcomes table diff --git a/R/NewDataConversion.R b/R/NewDataConversion.R index 17424a40..1f131c8d 100644 --- a/R/NewDataConversion.R +++ b/R/NewDataConversion.R @@ -323,7 +323,7 @@ convertToCyclopsData.tbl_dbi <- function(outcomes, } if (modelType == "pr" | modelType == "cpr") { - if (any((select(outcomes, time) %>% pull()) <= 0)) { + if (any(pull(outcomes, .data$time) <= 0)) { stop("time cannot be non-positive", call. = FALSE) } } diff --git a/R/TimeEffects.R b/R/TimeEffects.R index 5ae7aa0e..3bbdb6b5 100644 --- a/R/TimeEffects.R +++ b/R/TimeEffects.R @@ -22,8 +22,8 @@ splitTime <- function(shortOut, cut) { if (!"y" %in% colnames(shortOut)) stop("Must provide observed event status.") if ("rowId" %in% colnames(shortOut)) { shortOut <- shortOut %>% - rename(subjectId = rowId) %>% - arrange(subjectId) + rename(subjectId = .data$rowId) %>% + arrange(.data$subjectId) } else { shortOut <- shortOut %>% mutate(subjectId = row_number()) @@ -36,10 +36,10 @@ splitTime <- function(shortOut, cut) { episode = "stratumId", id = "newSubjectId")) longOut <- longOut %>% - rename(y = event) %>% - mutate(time = tstop - tstart) %>% - select(-c(newSubjectId, tstart, tstop)) %>% - arrange(stratumId, subjectId) + rename(y = .data$event) %>% + mutate(time = .data$tstop - .data$tstart) %>% + select(-c(.data$newSubjectId, .data$tstart, .data$tstop)) %>% + arrange(.data$stratumId, .data$subjectId) # Restore rowIds SubjectIds <- shortOut$subjectId @@ -49,9 +49,10 @@ splitTime <- function(shortOut, cut) { # Reorder columns longOut <- longOut %>% - select(rowId, everything()) %>% - select(subjectId, everything()) %>% - select(stratumId, everything()) + select(.data$stratumId, .data$subjectId, .data$rowId, everything()) + # select(.data$rowId, everything()) %>% + # select(.data$subjectId, everything()) %>% + # select(.data$stratumId, everything()) return(longOut) } diff --git a/man/appendSqlCyclopsData.Rd b/man/appendSqlCyclopsData.Rd index be922eff..6b9547c3 100644 --- a/man/appendSqlCyclopsData.Rd +++ b/man/appendSqlCyclopsData.Rd @@ -37,7 +37,7 @@ appendSqlCyclopsData( } \details{ Append data using two tables. The outcomes table is dense and contains ... The covariates table is sparse and contains ... -All entries in the outcome table must be sorted in increasing order by {oStratumId, oRowId}. All entries in the covariate table -must be sorted in increasing order by {cRowId}. Each cRowId value must match exactly one oRowId value. +All entries in the outcome table must be sorted in increasing order by (oStratumId, oRowId). All entries in the covariate table +must be sorted in increasing order by (cRowId). Each cRowId value must match exactly one oRowId value. } \keyword{internal} diff --git a/src/cyclops/CcdInterface.cpp b/src/cyclops/CcdInterface.cpp index d07c1330..b3070148 100644 --- a/src/cyclops/CcdInterface.cpp +++ b/src/cyclops/CcdInterface.cpp @@ -29,7 +29,7 @@ #include "CyclicCoordinateDescent.h" #include "ModelData.h" -#include "boost/iterator/counting_iterator.hpp" +//#include "boost/iterator/counting_iterator.hpp" #include "Thread.h" // #include "io/InputReader.h" @@ -373,9 +373,9 @@ double CcdInterface::profileModel(CyclicCoordinateDescent *ccd, AbstractModelDat } ); } else { - auto scheduler = TaskScheduler >( - boost::make_counting_iterator(0), - boost::make_counting_iterator(static_cast(bounds.size())), + auto scheduler = TaskScheduler>( + IncrementableIterator(0), // boost::make_counting_iterator(0), + IncrementableIterator(bounds.size()), //boost::make_counting_iterator(static_cast(bounds.size())), nThreads); auto oneTask = [&getBound, &scheduler, &ccdPool, &bounds](unsigned long task) { @@ -587,8 +587,10 @@ double CcdInterface::evaluateProfileModel(CyclicCoordinateDescent *ccd, Abstract values[i] = evaluate(points[i], ccd); } } else { - auto scheduler = TaskScheduler>( - boost::make_counting_iterator(0), boost::make_counting_iterator(static_cast(points.size())), nThreads); + auto scheduler = TaskScheduler>( + IncrementableIterator(0), //boost::make_counting_iterator(0), + IncrementableIterator(points.size()), //boost::make_counting_iterator(static_cast(points.size())), + nThreads); auto oneTask = [&evaluate, &scheduler, &ccdPool, &points, &values](unsigned long task) { values[task] = evaluate(points[task], ccdPool[scheduler.getThreadIndex(task)]); diff --git a/src/cyclops/CyclicCoordinateDescent.cpp b/src/cyclops/CyclicCoordinateDescent.cpp index 25145e1b..b2cf992b 100644 --- a/src/cyclops/CyclicCoordinateDescent.cpp +++ b/src/cyclops/CyclicCoordinateDescent.cpp @@ -208,12 +208,12 @@ void CyclicCoordinateDescent::init(bool offset) { // initialize starting betas to default value 0.0 startingBeta.resize(J, static_cast(0.0)); - + // hXBeta.resize(K, static_cast(0.0)); // hXBetaSave.resize(K, static_cast(0.0)); fixBeta.resize(J, false); - + hWeights.resize(0); cWeights.resize(0); // ESK: For censor weights @@ -757,7 +757,7 @@ void CyclicCoordinateDescent::kktSwindle(const ModeFindingArguments& arguments) int swindleIterationCount = 1; // int initialActiveSize = activeSet.size(); - int perPassSize = arguments.swindleMultipler; + // int perPassSize = arguments.swindleMultipler; while (!done) { @@ -885,7 +885,7 @@ void CyclicCoordinateDescent::kktSwindle(const ModeFindingArguments& arguments) } } ++swindleIterationCount; - perPassSize *= 2; + // perPassSize *= 2; logger->yield(); // This is not re-entrant safe } diff --git a/src/cyclops/Iterators.h b/src/cyclops/Iterators.h index 9b16022f..de82c796 100644 --- a/src/cyclops/Iterators.h +++ b/src/cyclops/Iterators.h @@ -1,7 +1,8 @@ #ifndef ITERATORS_H #define ITERATORS_H -#include +//#include +#include #include "CompressedDataMatrix.h" @@ -14,6 +15,47 @@ namespace bsccs { * single run-time determined iterator */ +template +class IncrementableIterator { +public: + + using iterator_category = std::forward_iterator_tag; + using value_type = T; + using difference_type = T; + using pointer = T*; + using reference = T&; + + IncrementableIterator(T value) : value(value) { } + + IncrementableIterator& operator++() { + ++value; + return *this; + } + + T operator*() const { return value; } + + bool operator==(const IncrementableIterator& rhs) const { + return value == rhs.value; + } + + bool operator!=(const IncrementableIterator& rhs) const { + return !(*this == rhs); + } + + template + IncrementableIterator operator+(V n) const { + IncrementableIterator copy = *this; + copy.value += n; + return copy; + } + + bool operator<(const IncrementableIterator& rhs) const { + return value < rhs.value; + } + +private: + T value; +}; struct IndicatorTag {}; struct SparseTag {}; @@ -31,7 +73,7 @@ class IndicatorIterator { typedef IndicatorTag tag; typedef int Index; typedef Scalar ValueType; - typedef boost::tuples::tuple XTuple; + typedef std::tuple XTuple; const static std::string name; @@ -123,7 +165,8 @@ class SparseIterator { typedef SparseTag tag; typedef int Index; typedef Scalar ValueType; - typedef boost::tuples::tuple XTuple; +// typedef boost::tuples::tuple XTuple; + typedef std::tuple XTuple; const static std::string name; @@ -300,7 +343,8 @@ class DenseIterator { typedef DenseTag tag; typedef int Index; typedef Scalar ValueType; - typedef boost::tuples::tuple XTuple; +// typedef boost::tuples::tuple XTuple; + typedef std::tuple XTuple; const static std::string name; @@ -392,7 +436,8 @@ class InterceptIterator { typedef InterceptTag tag; // TODO Fix!!! typedef int Index; typedef Scalar ValueType; - typedef boost::tuples::tuple XTuple; +// typedef boost::tuples::tuple XTuple; + typedef std::tuple XTuple; const static std::string name; diff --git a/src/cyclops/ModelData.cpp b/src/cyclops/ModelData.cpp index 9f72c2da..a75eb59b 100644 --- a/src/cyclops/ModelData.cpp +++ b/src/cyclops/ModelData.cpp @@ -17,8 +17,8 @@ #include #include -#include -#include +// #include +// #include #include "ModelData.h" diff --git a/src/cyclops/drivers/AbstractCrossValidationDriver.cpp b/src/cyclops/drivers/AbstractCrossValidationDriver.cpp index c0bab20b..ac6e48c3 100644 --- a/src/cyclops/drivers/AbstractCrossValidationDriver.cpp +++ b/src/cyclops/drivers/AbstractCrossValidationDriver.cpp @@ -7,8 +7,9 @@ #include #include +#include -#include "boost/iterator/counting_iterator.hpp" +// #include "boost/iterator/counting_iterator.hpp" #include "Types.h" #include "Thread.h" @@ -222,10 +223,15 @@ double AbstractCrossValidationDriver::doCrossValidationStep( auto& weightsExclude = this->weightsExclude; auto& logger = this->logger; - auto scheduler = TaskScheduler( - boost::make_counting_iterator(0), - boost::make_counting_iterator(arguments.foldToCompute), - nThreads); + // auto scheduler = TaskScheduler( + // boost::make_counting_iterator(0), + // boost::make_counting_iterator(arguments.foldToCompute), + // nThreads); + + auto scheduler = TaskScheduler>( + IncrementableIterator(0), + IncrementableIterator(arguments.foldToCompute), + nThreads); auto oneTask = [step, coldStart, nThreads, &ccdPool, &selectorPool, diff --git a/src/cyclops/drivers/AutoSearchCrossValidationDriver.cpp b/src/cyclops/drivers/AutoSearchCrossValidationDriver.cpp index 304ba85c..181b9264 100644 --- a/src/cyclops/drivers/AutoSearchCrossValidationDriver.cpp +++ b/src/cyclops/drivers/AutoSearchCrossValidationDriver.cpp @@ -21,7 +21,7 @@ #include "AbstractSelector.h" #include "../utils/HParSearch.h" -#include "boost/iterator/counting_iterator.hpp" +// #include "boost/iterator/counting_iterator.hpp" namespace bsccs { @@ -115,7 +115,7 @@ MaxPoint AutoSearchCrossValidationDriver::doCrossValidationLoop( // Newly re-located code double pointEstimate = doCrossValidationStep(ccd, selector, allArguments, step, nThreads, ccdPool, selectorPool, - predLogLikelihood); + predLogLikelihood); double stdDevEstimate = computeStDev(predLogLikelihood, pointEstimate); /* diff --git a/src/cyclops/engine/ModelSpecifics.h b/src/cyclops/engine/ModelSpecifics.h index 884291b7..52db0af7 100644 --- a/src/cyclops/engine/ModelSpecifics.h +++ b/src/cyclops/engine/ModelSpecifics.h @@ -1853,7 +1853,7 @@ struct TupleXGetterNew { template inline ReturnType operator()(TupleType& tuple) const { - return boost::get(tuple); + return std::get(tuple); } }; @@ -1885,7 +1885,7 @@ struct TestNumeratorKernel { template NumeratorType operator()(const NumeratorType lhs, const TupleType tuple) { - const auto expXBeta = boost::get<0>(tuple); + const auto expXBeta = std::get<0>(tuple); const auto x = getX(tuple); //boost::get<1>(tuple); return { @@ -1909,8 +1909,8 @@ struct TestGradientKernel { template GradientType operator()(const GradientType lhs, const NumeratorType numerator, const TupleType tuple) { - const auto denominator = boost::get<0>(tuple); - const auto weight = boost::get<1>(tuple); + const auto denominator = std::get<0>(tuple); + const auto weight = std::get<1>(tuple); return BaseModel::template incrementGradientAndHessian( diff --git a/src/cyclops/engine/ModelSpecifics.hpp b/src/cyclops/engine/ModelSpecifics.hpp index 8491689c..cf845ad0 100644 --- a/src/cyclops/engine/ModelSpecifics.hpp +++ b/src/cyclops/engine/ModelSpecifics.hpp @@ -19,7 +19,7 @@ #include "Recursions.hpp" #include "ParallelLoops.h" -#include "Ranges.h" +// #include "Ranges.h" #ifdef USE_RCPP_PARALLEL #include diff --git a/src/cyclops/engine/ParallelLoops.h b/src/cyclops/engine/ParallelLoops.h index bb6acf15..6333160f 100644 --- a/src/cyclops/engine/ParallelLoops.h +++ b/src/cyclops/engine/ParallelLoops.h @@ -5,7 +5,7 @@ #include #include #include -#include +// #include // #define USE_RCPP_PARALLEL #undef USE_RCPP_PARALLEL