forestry-labs / rforestry Goto Github PK

We're interested in compatibility with sklearn estimators (see instructions) for integration with other libraries like Ray. The goal would be to pass the check_estimator() check. A few specific items to respect requirements include: class __init__ parameters should be explicit and changing the newdata arg to x , adhered by subsequent methods.

Until we resolve this, we need to use ghcr.io/theo-s/rforestry/python_build_env:latest instead ghcr.io/forestry-labs/rforestry/python_build_env:latest.

treelite is a format for serializing trees for prediction only. It takes xgboost, lightgbm, and sklearn trees out of the box. It basically copies the internal structure of a tree, converts it into blazingly fast C code and makes predicting through the final structure extremely fast. There's also a CUDA treelite wrapper that converts any treelite model into one that works on GPUs.

One can also construct a treelite model from a custom trained model. All one needs are the following details for each tree: split points, feature at split, numerical vs categorical, and leaf nodes.

If we can get Rforestry to dump its internal structure into a JSON or something like that, I can work with that to convert it into a treelite tree. That'd give us most prediction performance perks + sklearn perks.

@JasjeetSekhon @theo-s

When plotting a model with linFeats = TRUE, I get the error: Error in elnet(x, is.sparse, ix, yx, y, weights, offset, type.gaussian, y is constant; gaussian glment fails at the standardization step

Here is code that produces it, largely taken from an example in ?forestry.

n <- c(100)
a <- rnorm(n)
b <- rnorm(n)
c <- rnorm(n)
y <- 4*a + 5.5*b - .78*c
x <- data.frame(a,b,c)

forest <- forestry(
  x,
  y,
  ntree = 10,
  replace = TRUE,
  maxDepth = 100,
  nodesizeStrictSpl = 1,
  nodesizeStrictAvg = 1,
  nthread = 2,
  linear = TRUE
)

plot(forest)

Can you fix function definition here

    def _get_weights_variables(self, weights: np.ndarray) -> np.ndarray:
        weights_variables = [i for i in range(weights.size) if weights[i] > max(weights) * 0.001]
        if len(weights_variables) < self.mtry:
            raise ValueError("mtry is too large. Given the feature weights, can't select that many features.")

        weights_variables = np.array(weights_variables, dtype=np.ulonglong)
        return weights_variables

for cases where weights is None (which is default value in fit() for feature_weights and deep_feature_weights) ?
Also, function argument weight should be Optional[np.ndarray]

@edwardwliu

👋 @petrovicboban

Please update the branch name bptest/test to the approved regex naming convention format below.

Regex: ^[a-z]{2,3}/.*$
Flags: i

Forestry saving and loading must be modified in the new branch in order to work with an averaging set different from the set used to construct the tree skeleton.

The predict function requires that feature.new to be provided. This should not be necessary.

It appears when P and N are both large, getVI has an abnormally long runtime.

The idea is:
(1) train a forest with a smallish number of trees
(2) see how often each feature is split on in this forest
(3) train a new forest with feature weights in proportion to the probabilities each feature was split on in the original forest
(4) combine the trees from (1) and (3) into single forest to produce a final prediction.

This idea arose from a conversation with Jas.

The order of the arguments for getOOBpreds should be changed to:

getOOBpreds(object, feature.new = NULL, noWarning)

from:

getOOBpreds(object, noWarning, feature.new = NULL)

In common usage people are going to call it with:

getOOBpreds(objects, new.features)

and run into trouble.

Support exporting trained models to JSON. The preferred spec should be compatible with Treelite, see documentation. Since serialization can be relatively slow as well as need to be supported in R and Python, I would consider exposing a C++ method for the JSON export. Otherwise, if we change any underlying model representations, we would need to update the wrappers separately. cc @theo-s.

Missing data Y values need to be taken into account by monotonic constraints.

Hey @theo-s if I'm understanding the code correctly, predictedMean below isn't getting re-calculated for leaf nodes right?

Since getAveragingIndex() always returns the same index and (*trainingData) doesn't change. In that case, predictedMean should just be calculated for each leaf node during training and accessed

Please support decision path in predict method for random forest.
This helps in understanding proximity between observations (for deeper trees, terminal nodes might not hold much)

Reference: https://scikit-learn.org/stable/modules/generated/sklearn.tree.DecisionTreeClassifier.html (see 'decision_path')

• pytest-sugar

While implementing saving + loading in Python, I realized that that we don't have customAveragingSample, customSplittingSample, and customExcludedSample implemented in the Python wrapper. It would be useful to have these, but I will not implement them in the saving + loading PR.

Let's have

form random_forestry import RandomForest

rf_1 = RandomForest()
rf_2 = RandomForest()

Since default value for seed is random number, we expect:

assert rf_1.seed != rf_2.seed

but that's not what's happening.

although rf_1 and rf_2 are really different objects:

assert id(rf_1) != id(rf_2)

their attributes which have default value set are not:

assert id(rf_1.seed) == id(rf_2.seed)

Python does copy on write here, so if value is changed later, it will create new seed object then:

rf_1.seed = 5
assert id(rf_1.seed) != id(rf_2.seed)

The implication is that if you create different RandomForest objects with seed not explicitly set, seed will be random but all objects will have the same value.
Is this acceptable to you? @edwardwliu @theo-s

Trying to test the package. Despite proper download & install, I keep on getting the [could not find function "forestry"] error message. Thanks !

preprocessing.py has method has_nas, but it's not used anywhere. Requires cleanup.

When passing a data frame with 0 in training, the resulting processed_x occasionally replaces 0 with 1.387e-17. This is
likely due to floating point behavior either in the C++ logic or wrapper APIs. This has also led to flakiness in some exact tests.

Several parts of the Python package README are outdated, and this is used for the page on PyPI.

https://pypi.org/project/random-forestry/

Description should be cleaner, and link to the official documentation site.

Currently when both minTreesPerGroup and ntree are set in the branch group_sampling, ntree will be overridden. Let's modify it so that it uses ntree = max(ntree, number of trees we need to satisfy minTreesPerGroup option).

R is not required any more in python build environment image, it should be removed.

Currently forestry errors out cryptically with a colSd error when a column in the training data contains only NAs. It should either make that error more informative or just drop the column from its training data.

When tuning a random forest using the caret package, segmentation faults occur often when splitratio = 0.5.

For a reproducible example, see: https://github.com/forestry-labs/GI-Collaboration/blob/main/code/3-Modeling/3.2-runPipeline.R

A segfault will occur with the parameters that are currently set:

dataset = "ehrbase"
estimator = "randomforest"
predictor = estimator
seed = 101
force = 0
embedding_dict = NULL
vi = 0

When running in a bash script, the error message would look something like:

• Fold7.Rep1: mtry=12, nodesizeStrictSpl=2, ntree=500, splitratio=0.5521

• Fold7.Rep1: mtry=12, nodesizeStrictSpl=4, ntree=500, splitratio=0.5000
  mtry nodesizeStrictSpl ntree splitratio
  4 12 4 500 0.5

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address 0x8e0, cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'unknown'

*** caught segfault ***

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address 0x448, cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address 0x88f, cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'memory not mapped'
Error: no more error handlers available (recursive errors?); invoking 'abort' restart

*** caught segfault ***
address (nil), cause 'memory not mapped'

*** caught segfault ***
address (nil), cause 'unknown'
An irrecoverable exception occurred. R is aborting now ...
An irrecoverable exception occurred. R is aborting now ...

We need to get monotonicity constraints working with the missing data implementation.

Are we sure that this is a good name for the argument in predict? For example, how about newx as in glmnet?

In some cases, it appears that the runtime of relinkCPP_ptr is scaling nonlinearly in the number of trees in the forest.

Add flag to R predict wrapper in order to aggregate tree predictions in a standardized way.

We need to pin versions of all build and runtime python dependencies so we can have consisting build process.

We should make predict be deterministic even with threading and aggregation=doubleOOB and OOB. Just create one index, and then sum over missing indices.

Here's an example. Note that this code will cause R to crash.

n <- c(100)
a <- rnorm(n)
b <- rnorm(n)
c <- rnorm(n)
y <- 4*a + 5.5*b - .78*c
x <- data.frame(a,b,c)

forest <- forestry(
  x,
  y,
  ntree = 10,
  replace = TRUE,
  maxDepth = 100,
  nodesizeStrictSpl = 10,
  nodesizeStrictAvg = 10,
  nthread = 2,
  linear = TRUE,
  linFeats = 1
)

predict(forest, aggregation = "oob")

Running relinkCPP_prt in parallel fails. I think it is caused by this issue:
https://cran.r-project.org/web/packages/future/vignettes/future-4-non-exportable-objects.html

Here is an example:

library(Rforestry)
library(parallel)
n <- 100
p <- 2
x <- matrix(rnorm(n * p), ncol = p)
y <- rnorm(n)

forest1 <- make_savable(forestry(x, y))
forest2 <- make_savable(forestry(x, y))

saveRDS(forest1, file = "forest1.RDS")
saveRDS(forest2, file = "forest2.RDS")

forest1 <- readRDS("forest1.RDS")
forest2 <- readRDS("forest2.RDS")

# Reconstruction fails here:
relinked <- mclapply(list(forest1, forest2), relinkCPP_prt, mc.cores = 2)
relinked[[1]]@forest


# Checking that it works serially:
relinkCPP_prt(forest1)@forest

Should add a seed parameter to corrected predict in order to pass to the forestry calls so that corrected predictions can be deterministic

Tests show the loadForestry uses all of the CPU even when nthread != 0. It may be good to add a thread option to loadForestry (just like predict).

Both README.md and Sphinx docs are outdated.

Would be good to change this so we check for the error in R code and print a clearer error message. Small reproducible example:

n <- 10
p <- 10
# Generate Data
x <- matrix(runif(n*p,min=-2,max=2), ncol=p)
y <- x[,1]**3
colnames(x) <- c("V1","V2","V3","V4","V5","V6","V7","V8","V9","V10")
# Regress
rf <- forestry(x=x,
               y=y,
               ntree=500,
               seed=seed_i,
               OOBhonest = TRUE,
               scale = FALSE,
               monotonicConstraints = c(1,1,-1,rep(0,7)),
               monotoneAvg = TRUE,
               symmetric = TRUE
)

Output:

<simpleError in rcpp_cppBuildInterface(processed_x, y, categoricalFeatureCols_cpp,     linFeats, nObservations, numColumns, ntree, replace, sampsize,     mtry, splitratio, OOBhonest, doubleBootstrap, nodesizeSpl,     nodesizeAvg, nodesizeStrictSpl, nodesizeStrictAvg, minSplitGain,     maxDepth, interactionDepth, seed, nthread, verbose, middleSplit,     maxObs, featureWeights, featureWeightsVariables, deepFeatureWeights,     deepFeatureWeightsVariables, observationWeights, monotonicConstraints,     groupVector, minTreesPerGroup, monotoneAvg, hasNas, linear,     symmetric, overfitPenalty, doubleTree, TRUE, rcppDataFrame): object 'seed_i' not found>

This is a bit of a weird edge case because we're passing a variable name seed_i that does not exist in the calling environment.

If groups is shorter than Nobs, we get a core dump when using the groups option.

@theo-s this is the error you wanted me to look at right?

function no longer works

• Sometimes a potential split appears to break monotonicity but if there were no precision issues it would not. To avoid this issue consider implementing a threshold that permits splits that break monotonicity provided they do not break it by more than can be plausibly explained by the accumulation of imprecision. This should be a flag settable in the R call to forestry in case we want guaranteed monotonicity. This will only come into play when only poor (in terms of mse reduction) splits are available but below the poor split they make be better splits if better features are in the subsample of the features, etc.
• Scale and center the data before any computation is done. This is done in other packages, eg glmnet, as it improves precision. It should only make a difference for the outcome as the forest only depends on the ranks of the features. If it does change the ranks of the features that is probably a negative. Before doing this we need to make sure it doesn't affect any of the other options, eg enforcement of monotonicity, but I think it should be fine.

We may want to remove scaling entirely, but currently the predictions are not being rescaled (for any aggregation options when the forest is trained with scale = True).

A minimum reproducible example is as follows:

import numpy as np
import pandas as pd
from sklearn.datasets import load_iris
from sklearn.metrics import mean_squared_error
from random_forestry import RandomForest

data = load_iris()
X = pd.DataFrame(data["data"], columns=data["feature_names"])
y = data["target"]

# Create a RandomForest object
fr = RandomForest(ntree=100, max_depth=5, seed=1,oob_honest = True,scale=False)

fr.fit(X.iloc[:, 1:], X.iloc[:, 0])

print("Aggregation = average")
print(np.sqrt(mean_squared_error(X.iloc[:, 0],fr.predict(X.iloc[:, 1:], aggregation="average", exact = True))))
# Predictions are not on the scale of 1st column of iris, should be scaled + centered
print(fr.predict(X.iloc[:, 1:], aggregation="average", exact = True)[[0,1,50,51,100,101]])