Skip to content

Merge ICP-Flow into codebase - #15

Merged
Kin-Zhang merged 12 commits into
mainfrom
feature/icpflow
Nov 21, 2025
Merged

Merge ICP-Flow into codebase#15
Kin-Zhang merged 12 commits into
mainfrom
feature/icpflow

Conversation

@Kin-Zhang

@Kin-Zhang Kin-Zhang commented Aug 11, 2025

Copy link
Copy Markdown
Member

This pull request integrates support for the ICP-Flow model into the OpenSceneFlow codebase. It adds the ICP-Flow method as a new optimization-based scene flow estimation approach, provides documentation and configuration for its usage, and incorporates its CUDA-accelerated histogram library for efficient computation. The changes also update the project documentation to reflect the new method and its requirements.

ICP-Flow Model Integration

  • Added ICP-Flow model implementation, configuration (conf/model/icpflow.yaml), and import logic in src/models/__init__.py, with instructions for required dependencies (pytorch3d, custom CUDA histogram library). [1] [2]

CUDA Histogram Library Addition

  • Introduced a new CUDA-accelerated histogram library (assets/cuda/histlib) with source files (hist.cu, hist_cuda_core.cuh, hist_cuda.cpp, hist_cuda.h, hist.h, setup.py, and Python bindings in __init__.py) for efficient ICP-Flow computations. [1] [2] [3] [4] [5] [6] [7]

The merge is ongoing. More details will be updated here. Done. Check AI Summary above.

Original source version: https://github.com/yanconglin/ICP-Flow; The cleanup codebase version is from the HiMo project benchmark.

Inside our OpenSceneFlow codebase, it can support ICP-Flow with multi-GPUs running. More detail can be found in the first merge of optimization-based methods.

@Kin-Zhang
Kin-Zhang requested a review from yanconglin August 11, 2025 20:15
@Kin-Zhang Kin-Zhang added the new method new method involve label Aug 11, 2025
* conf(optimization-based): update all config files.
* todo: update model file, double check with yancong and Qingwen confirm that: icp-flow results can be reproduced and tested.
@Kin-Zhang

Copy link
Copy Markdown
Member Author

Codes have all been updated, waiting for @yanconglin's review approval. After approval, I will squash and merge into the main branch.

@yanconglin

Copy link
Copy Markdown
Collaborator

Hi Kin,

Thanks a lot for setting up OpenSceneFlow—great work for the community!
Looks like the team put a lot of work into optimizing the code.
I’ll dig into the details and share updates once I make some progress.

Kind regards,
Yancong

@Kin-Zhang

Copy link
Copy Markdown
Member Author

Hi, just want to sync the progress (after 1 month). If it's not possible to review, as you may be busy and hard to review afterward, I can change it to reproduce rather than highlighting in the README. Let me know if you have time to review.

@yanconglin

Copy link
Copy Markdown
Collaborator

Hi, I spent some time reviewing the code. During the process, I realized there’s plenty of room for improvement—mainly in computation—by migrating all components to CUDA (partially inspired by your great ChamferDistance with shared memory). I started working in this direction, but the workload turned out to be more substantial than I expected. So, I’m thinking it might be better to roll back for now. Do you have an expected timeline in mind for the merging?

@Kin-Zhang

Copy link
Copy Markdown
Member Author

Hi, Thanks for your reply, glad to hear it could be improved, and it could be interesting to the community. If the improvement takes longer, maybe we can have another pull request afterward.

For this merging, I expected to do it within this year (before 2026). So users can easily run all baselines, etc. How do you think?

As the official review didn't finish, will open another request once Yancong have time to review/improve.
* update master port also for multi-program run.
@Kin-Zhang

Kin-Zhang commented Nov 21, 2025

Copy link
Copy Markdown
Member Author

Regarding the HiMo leaderboard baseline setup, I have temporarily moved ICP-Flow to the unofficial category.
Once you've addressed the code review or finalized the improvements, please go ahead and open a new pull request.
Thanks again for your time and discussion.

revert to previous fix. successfully run on all opt method.
@Kin-Zhang
Kin-Zhang merged commit 88f49bc into main Nov 21, 2025
@Kin-Zhang
Kin-Zhang deleted the feature/icpflow branch November 21, 2025 11:50
@yanconglin
yanconglin restored the feature/icpflow branch November 25, 2025 12:02
@yanconglin

yanconglin commented Nov 26, 2025

Copy link
Copy Markdown
Collaborator

Version 1 Metric on EPE Three-way:


EPE_FD 0.173469
EPE_BS 0.0305864
EPE_FS 0.0377146
IoU 0.251697
Three-way 0.0805902


Version 2 Metric on Normalized Category-based:
| Class | Static | Dynamic |
|----------------+-----------+------------|
| BACKGROUND | 0.0269729 | nan |
| CAR | 0.0387519 | 0.400381 |
| OTHER_VEHICLES | 0.02472 | 0.458639 |
| PEDESTRIAN | 0.0234094 | 0.424822 |
| WHEELED_VRU | 0.022222 | 0.334968 |
| Mean | 0.0272152 | 0.404702 |

Version 3 Metric on EPE Distance-based:
| Distance | Static | Dynamic | #Static | #Dynamic |
|------------+----------+-----------+---------------+------------------|
| 0-35 | 0.0282 | 0.2265 | 2.88731e+08 | 1.58288e+07 |
| 35-50 | 0.0434 | 0.3085 | 4.02818e+07 | 674520 |
| 50-75 | 0.0462 | 0.4299 | 8.90925e+06 | 58742 |
| 75-100 | 0.0104 | [] | 191 | 0 |
| 100-inf | [] | [] | 0 | 0 |
| Mean | 0.032 | 0.3216 | nan | nan |

The above table shows the ICPFlow results (OpenSceneFlow implementation) on Argoverse 2 val set.
My observations are:
(1) Three-way EPE shows marginal difference compared to the reported values in the paper.
(2) Bucketed EPE shows outstanding difference, especially on the car category (out of my expectation).

So far, it seems NOT possible to reproduce the leaderboard result at this moment.
Why is this the case?
Ground removal plays a role. ICP-Flow was designed to take as input full lidar scans and employs Patchwork++ to remove ground points. However, the Argoverse 2 and Waymo benchmarks provide labels for ground/non-ground points, so there is no need to perform ground removal. More importantly, the HDBSCAN algorithm may behave differently when ground points are not well removed (kind of inevitably painful when you do feature engineering :( ).
Ground removal is perhaps a major cause that leads to the difference. To fully reproduce the result, one might have to include the Patchwork++ pipeline into the codebase, which seems to be too much effort.

Additional remark: I have always been cautious about the ground removal procedures introduced in the argo/waymo benchmarks, as they use some HD maps info or annotation info. This can be a barrier when deploying scene flow models in reality (e.g., only lidar scans available). For this reason, I would rather make ground removal also a part of scene flow model to benchmark their ability in real scenarios, though ground points provide little (almost no) useful info about motion.

Snapshot of the result on argo-2 val attached.
Screenshot 2025-11-26 at 10 10 37

yanconglin added a commit that referenced this pull request Nov 26, 2025
@Kin-Zhang

Kin-Zhang commented Nov 26, 2025 via email

Copy link
Copy Markdown
Member Author

@yanconglin

yanconglin commented Nov 26, 2025

Copy link
Copy Markdown
Collaborator

Ground removal here for every method is same I think it’s more fair

Yep, all learning models take the same ground-removed points as input. This is "fair" for comparison among them.
However, again, I would argue that this is not a good practice to manually remove the ground points beforehand in reality.
There is perhaps a need to properly evaluate the ground removal as well, e.g., how many instance points are chopped off because of incorrect ground removal.

Ground removal matters more for ICP Flow due to its fully handcrafted nature. linefit does not work that well and often leaves a large piece of ground unremoved, and therefore, HDBSCAN fails to cluster individual instances in the later stage. This has an (to what extent?) impact on the final result. But anyway, ICP Flow has its drawbacks, and it is important to point this out.

@yanconglin

Copy link
Copy Markdown
Collaborator

Again, I do not see significant bugs in the code at this moment, and I do like the way it is structured. I am afraid it is perhaps too much effort to reproduce the identical result.

@Kin-Zhang
Kin-Zhang deleted the feature/icpflow branch November 27, 2025 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new method new method involve

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants