Merge branch 'alex-removeJRI'

BardyBard · BardyBard · commit 4f580c47bb49 · 2025-04-10T15:42:50.000-04:00
diff --git a/java-r5rcore/src/main/resources/logback.xml b/java-r5rcore/src/main/resources/logback.xml
@@ -0,0 +1,25 @@
+<configuration>
+
+    <!-- Define LOG_PATH using DATA_PATH with fallback to current dir -->
+    <property name="LOG_PATH" value="${LOG_PATH:-./r5r-log.log}" />
+
+    <appender name="RFILE" class="ch.qos.logback.core.FileAppender">
+        <file>${LOG_PATH}</file>
+        <encoder>
+            <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger - %msg%n</pattern>
+        </encoder>
+        <append>false</append>
+    </appender>
+
+    <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger - %msg%n</pattern>
+        </encoder>
+    </appender>
+
+    <root level="ALL">
+        <appender-ref ref="CONSOLE"/>
+        <appender-ref ref="RFILE"/>
+    </root>
+
+</configuration>
diff --git a/java-r5rcore/src/org/ipea/r5r/Process/R5Process.java b/java-r5rcore/src/org/ipea/r5r/Process/R5Process.java
@@ -12,6 +12,8 @@
 import org.ipea.r5r.RDataFrame;
 import org.ipea.r5r.RoutingProperties;
 import org.ipea.r5r.Utils.Utils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.*;
 import java.text.ParseException;
@@ -61,6 +63,8 @@ public abstract class R5Process {
 
     protected abstract boolean isOneToOne();
 
+    private static final Logger LOG = LoggerFactory.getLogger("org.ipea.r5r.Process.R5RProcess");
+
     public R5Process(ForkJoinPool threadPool, TransportNetwork transportNetwork, RoutingProperties routingProperties) {
         this.r5rThreadPool = threadPool;
         this.transportNetwork = transportNetwork;
@@ -80,9 +84,7 @@ public RDataFrame run() throws ExecutionException, InterruptedException {
                         filter(Objects::nonNull).
                         collect(Collectors.toList())).get();
 
-        if (!Utils.verbose & Utils.progress) {
-            System.out.print(".. DONE!\n");
-        }
+        LOG.info(".. DONE!\n");
 
         RDataFrame results = mergeResults(processResults);
 
@@ -197,9 +199,8 @@ private RDataFrame tryRunProcess(AtomicInteger totalProcessed, int index) {
                 results.clear();
             }
 
-            if (!Utils.verbose & Utils.progress) {
-                System.out.print("\r" + totalProcessed.getAndIncrement() + " out of " + nOrigins + " origins processed.");
-            }
+            LOG.info("\r{} out of {} origins processed.", totalProcessed.getAndIncrement(), nOrigins);
+
         } catch (ParseException | FileNotFoundException e) {
             e.printStackTrace();
         }
@@ -224,9 +225,7 @@ private String getCsvFilename(int index) {
     protected abstract RDataFrame runProcess(int index) throws ParseException;
 
     private RDataFrame mergeResults(List<RDataFrame> processResults) {
-        if (!Utils.verbose & Utils.progress) {
-            System.out.print("Consolidating results...");
-        }
+        LOG.info("Consolidating results...");
 
         int nRows;
         nRows = processResults.stream()
@@ -252,9 +251,7 @@ private RDataFrame mergeResults(List<RDataFrame> processResults) {
         );
         mergedDataFrame.updateRowCount();
 
-        if (!Utils.verbose & Utils.progress) {
-            System.out.print(" DONE!\n");
-        }
+        LOG.info(" DONE!\n");
 
         return mergedDataFrame;
     }
diff --git a/java-r5rcore/src/org/ipea/r5r/R5RCore.java b/java-r5rcore/src/org/ipea/r5r/R5RCore.java
@@ -15,11 +15,8 @@
 import org.ipea.r5r.Network.NetworkBuilder;
 import org.ipea.r5r.Process.*;
 import org.ipea.r5r.Utils.Utils;
-import org.rosuda.JRI.RConsoleOutputStream;
-import org.rosuda.JRI.Rengine;
 import org.slf4j.LoggerFactory;
 
-import java.io.PrintStream;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.ParseException;
@@ -190,7 +187,7 @@ public void verboseMode() {
     }
 
     public void setProgress(boolean progress) {
-        Utils.progress = progress;
+        Utils.setlogProgress(progress);
     }
 
     public void setBenchmark(boolean benchmark) {
@@ -211,6 +208,8 @@ public String getOutputCsvFolder () {
         return Utils.outputCsvFolder;
     }
 
+    public String getLogPath() { return System.getProperty("LOG_PATH"); }
+
     public void setDetailedItinerariesV2(boolean v2) {
         Utils.detailedItinerariesV2 = v2;
     }
@@ -220,10 +219,6 @@ public void setDetailedItinerariesV2(boolean v2) {
     private static final org.slf4j.Logger LOG = LoggerFactory.getLogger(R5RCore.class);
 
     public R5RCore(String dataFolder, boolean verbose, String nativeElevationFunction) throws Exception {
-        Rengine r = new Rengine();
-        RConsoleOutputStream rs = new RConsoleOutputStream(r, 1);
-        System.setOut(new PrintStream(rs));
-
         if (verbose) {
             verboseMode();
         } else {
@@ -681,11 +676,6 @@ public boolean hasFrequencies() {
         return this.transportNetwork.transitLayer.hasFrequencies;
     }
 
-    public void message(String m) {
-        Rengine r = new Rengine();
-        r.eval("message(\"" + m + "\")");
-    }
-
     public void abort() {
         LOG.error("process aborted");
     }
diff --git a/java-r5rcore/src/org/ipea/r5r/Utils/Utils.java b/java-r5rcore/src/org/ipea/r5r/Utils/Utils.java
@@ -72,6 +72,22 @@ public static String getTimeFromSeconds(int secondsFromMidnight) {
         return (strHours + ":" + strMin + ":" + strSec);
     }
 
+    public static void setlogProgress(boolean progress){
+        Utils.progress = progress;
+
+        LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory();
+        Logger logger;
+        logger = loggerContext.getLogger("org.ipea.r5r.Process.R5RProcess");
+
+        if (Utils.verbose) {
+            logger.setLevel(Level.ALL);
+        } else if (progress) {
+            logger.setLevel(Level.INFO);
+        } else {
+            logger.setLevel(Level.ERROR);
+        }
+    }
+
     public static void setLogMode(String mode, boolean verbose) {
         Utils.verbose = verbose;
 
diff --git a/r-package/R/setup_r5.R b/r-package/R/setup_r5.R
@@ -69,10 +69,13 @@ setup_r5 <- function(data_path,
     stop("The 'elevation' parameter only accepts one of the following: c('TOBLER', 'MINETTI','NONE')")
     }
 
+  # expand data_path to full path, as required by rJava api call
+  data_path <- path.expand(data_path)
 
   # check Java version installed locally ---------------------------------------
-
-  rJava::.jinit()
+  log_filename <- paste0("r5rlog_", format(Sys.time(), "%Y%m%d"), ".log")
+  log_path <- file.path(data_path, log_filename)
+  rJava::.jinit(parameters = paste0("-DLOG_PATH=", log_path))
   ver <- rJava::.jcall("java.lang.System", "S", "getProperty", "java.version")
   ver <- as.numeric(gsub("\\..*", "", ver))
   if (ver != 21) {
@@ -85,9 +88,6 @@ setup_r5 <- function(data_path,
     )
   }
 
-  # expand data_path to full path, as required by rJava api call
-  data_path <- path.expand(data_path)
-
   # check if data_path has osm.pbf, .tif gtfs data, or a network.dat file
   any_network <- length(grep("network.dat", list.files(data_path))) > 0
   any_pbf  <- length(grep(".pbf", list.files(data_path))) > 0
diff --git a/r-package/inst/jar/r5r.jar b/r-package/inst/jar/r5r.jar
diff --git a/r-package/tests/testthat/test-set_progress.R b/r-package/tests/testthat/test-set_progress.R
@@ -44,16 +44,23 @@ test_that("progress argument works in routing functions", {
 
     progress_regex <- "\\d+ out of \\d+ origins processed\\."
 
-    progress_messages <- capture.output(
-      res <- eval(parse(text = progress_expr)),
-      type = "message"
-    )
+
+    log_file <- file.path(r5r_core$getLogPath())
+    # Clean log before test
+    if (file.exists(log_file)) writeLines("", log_file)
+
+    res <- eval(parse(text = progress_expr))
+    # Wait a bit to ensure Java flushed the logs
+    Sys.sleep(0.2)
+    progress_messages <- readLines(log_file)
     expect_true(any(grepl(progress_regex, progress_messages)))
 
-    non_progress_messages <- capture.output(
-      res <- eval(parse(text = non_progress_expr)),
-      type = "message"
-    )
+    # reset log again
+    writeLines("", log_file)
+    res <- eval(parse(text = non_progress_expr))
+    # Wait a bit to ensure Java flushed the logs
+    Sys.sleep(0.2)
+    non_progress_messages <- readLines(log_file)
     expect_false(any(grepl(progress_regex, non_progress_messages)))
   }
 
diff --git a/r-package/tests/testthat/test-set_verbose.R b/r-package/tests/testthat/test-set_verbose.R
@@ -44,17 +44,24 @@ test_that("verbose argument works in routing functions", {
 
     info_regex <- "(\\[.*\\] INFO)|(\\[.*\\] DEBUG)|(\\[.*\\] WARN)"
 
-    verbose_messages <- capture.output(
-      res <- eval(parse(text = verbose_expr)),
-      type = "message"
-    )
-    expect_true(any(grepl(info_regex, verbose_messages)))
+    log_file <- file.path(r5r_core$getLogPath())
+    # Clean log before test
+    if (file.exists(log_file)) writeLines("", log_file)
 
-    non_verbose_messages <- capture.output(
-      res <- eval(parse(text = non_verbose_expr)),
-      type = "message"
-    )
+    res <- eval(parse(text = non_verbose_expr))
+    # Wait a bit to ensure Java flushed the logs
+    Sys.sleep(0.2)
+    non_verbose_messages <- readLines(log_file)
     expect_false(any(grepl(info_regex, non_verbose_messages)))
+
+    # reset log again
+    writeLines("", log_file)
+    res <- eval(parse(text = verbose_expr))
+    # Wait a bit to ensure Java flushed the logs
+    Sys.sleep(0.2)
+    verbose_messages <- readLines(log_file)
+
+    expect_true(any(grepl(info_regex, verbose_messages)))
   }
 
   assert_function(travel_time_matrix)
