initiating RCF3.0 #283
initiating RCF3.0 #283
Conversation
Java/core/src/main/java/com/amazon/randomcutforest/executor/ITraversable.java
Outdated
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/store/IntervalManager.java
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/store/IntervalManager.java
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/store/RCF3PointStoreLarge.java
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/executor/ITraversable.java
Outdated
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/store/IntervalManager.java
Outdated
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/store/PointStore.java
Outdated
Show resolved
Hide resolved
| */ | ||
| @Override | ||
| public int decrementRefCount(int index) { | ||
| // indexManager.checkValidIndex(index); |
There was a problem hiding this comment.
Should this be removed or uncommented?
There was a problem hiding this comment.
removed or instantiated to the current notion of valid index, next commit
Java/core/src/main/java/com/amazon/randomcutforest/store/RCF3PointStore.java
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/tree/AbstractNodeStore.java
Show resolved
Hide resolved
| return false; | ||
| } | ||
|
|
||
| public BoundingBoxFloat getBox(int index, IPointStoreView<float[]> pointStoreView) { |
There was a problem hiding this comment.
It seems strange to me to make the point store view a method parameter and not an instance field. By having it as a method argument, that suggests that you should be able to pass any point store view in and get some kind of answer.
There was a problem hiding this comment.
I think the 2.0 version called for a more lightweight nodeview -- let me revisit this, it's possible that we can simplify these.
Java/core/src/main/java/com/amazon/randomcutforest/tree/AbstractNodeStore.java
Outdated
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/tree/AbstractNodeStore.java
Outdated
Show resolved
Hide resolved
Java/core/src/main/java/com/amazon/randomcutforest/tree/AbstractNodeView.java
Outdated
Show resolved
Hide resolved
|
|
||
| public class RCF3PointStoreLarge extends RCF3PointStore { | ||
|
|
||
| static int INFEASIBLE_LOCN = (int) -1; |
There was a problem hiding this comment.
@sudiptoguha I think this bug #322 is caused by this line, right?
There was a problem hiding this comment.
It's not a bug. The representation of the pointstore changed :) Look at lines 28 and 31-37; the unused locations have value -1. However for the 2.0.1 models the unused valued could have been anything (the check was via refList). I think 1.3 picked up something earlier than 283.
Description of changes: This PR initiates RCF 3.0.
RCF 2.0 introduced multiple primitives, such as bounding box caching, tree representations, columnar compression to provide a space-time tradeoff. However those primitives were not optimized in any manner. RCF 3.0 reorganizes those primitives to provide a more optimal (and hopefully cleaner) tradeoff.
None of the visitor classes were changed and no inference is expected to vary in this PR. There is unlikely to be change in performance of RCF2.0 and RCF3.0 for large model sizes (bounding box cache 1). The RCF 2.0 files were not changed (except minor bug fixes and plumbing to allow direct comparison).
RCF3.0 will focus exclusively on single precision. Double precision calculations over randomized data structures is unlikely to be helpful in the context of RCF. Moreover the newer logic in RCF 3.0 literally "thinks outside the bounding box". While the bounding box description is easy to follow, relying on such descriptions inside an algorithm may be inimical to algorithmic efficiency. We note that efficiency is a key distinguishing feature of RCFs vis-a-vis random forests.
This PR also overhauls the Rust version to the same exact algorithm as the Java version. RCF 2.0 was designed with Rust and memory safely in mind. The parallel implementation underscores that point. We see the Rust version as a mechanism of additional verification of the algorithmic ideas and expect the different language implementations to remain in sync. None of these versions are superior to the other in performance, and we expect to continue to do everything to keep the Java version as performant as possible.