GPU Kalman Filter Implementation#20
Conversation
Summary of ChangesHello @spvio, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for a GPU-accelerated Kalman Filter, aiming to leverage parallel processing for improved performance. It involves adding a new compute shader kernel and updating build configurations. While the core GPU kernel file is being introduced, the provided patch also includes a commented-out section of GLSL-like code within a C++ parsing file, suggesting initial exploration or temporary storage of GPU-related logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a GPU Kalman Filter. My review of photon/parse/parse.cpp found a couple of issues. There's a duplicated header include which should be removed. More significantly, a large block of commented-out code has been added. This code should be removed to maintain codebase clarity, as version control is the proper place for historical or experimental code. The commented code also contains some internal issues like an unprofessional comment and undefined variables, reinforcing the recommendation for its removal.
| /*[λ] the photon parsing interface*/ | ||
| #include "parse.hpp" |
| // layout(local_size_x = 4) in; // 4 threads, one per row | ||
|
|
||
| // // Input | ||
| // layout(binding = 0) buffer InputMatrix { | ||
| // float A[16]; | ||
| // }; | ||
|
|
||
| // layout(binding = 1) buffer OutputMatrix { | ||
| // float L[16]; | ||
| // }; | ||
|
|
||
| // void main() { | ||
| // uint row = gl_GlobalInvocationID.x; // Thread-per-row!! | ||
|
|
||
|
|
||
| // const uint N = 4; | ||
|
|
||
| // for (uint col = 0; col < N; col++) { | ||
|
|
||
| // if (row < col) { | ||
|
|
||
| // } | ||
|
|
||
| // if (row == col) { | ||
| // // Diagonal shii element | ||
| // L[row * N + col] = sqrt(A[row * N + col] - sum); | ||
| // } | ||
| // else { | ||
|
|
||
| // L[row * N + col] = (A[row * N + col] - sum) / L[col * N + col]; | ||
| // } | ||
| // barrier(); // Synchronize? threads in this workgroup | ||
| // } | ||
| // } No newline at end of file |
There was a problem hiding this comment.
This large block of commented-out code should be removed. Code that is not in use should not be kept in comments, as it clutters the codebase and can be confusing for future developers. Version control is the appropriate tool for managing code history and experiments.
Furthermore, the commented code contains a comment on line 31 (// Diagonal shii element) that should be phrased more professionally, and it also uses an undefined variable sum.
Files Added
Files Modified