[GSPH] Add GSPH Model class and Python bindings #1454
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary of ChangesHello @Guo-astro, 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 significantly enhances the GSPH module by introducing a robust Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive GSPH (Godunov Smoothed Particle Hydrodynamics) module, including the core solver, configuration classes, and Python bindings. The overall structure is well-organized, and the use of modern C++ features like std::variant for configuration is commendable. The Python bindings are well-documented, which will greatly aid usability.
However, the review has identified several critical and high-severity issues that need to be addressed. These include potential out-of-bounds memory access due to missing input validation, use of hardcoded constants instead of configurable parameters, and inconsistencies in the implementation of boundary conditions. There are also significant performance concerns related to inefficient data handling, code duplication, and suboptimal particle loading from checkpoints. Addressing these points will be crucial for the stability, performance, and maintainability of this new module.
Guo-astro
force-pushed
the
gsph-bindings
branch
from
a7e1ed7 to
478996e
Compare
Guo-astro
force-pushed
the
gsph-bindings
branch
from
478996e to
5c56593
Compare
Workflow reportworkflow report corresponding to commit c3da280 Light CI is enabled. This will only run the basic tests and not the full tests. Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Doxygen diff with
|
Guo-astro
force-pushed
the
gsph-bindings
branch
from
635142b to
e03bde1
Compare
Adds main solver class and VTK output for GSPH: Solver.hpp/cpp: - Main GSPH solver orchestrating the simulation - Predictor-corrector time integration (leapfrog) - Ghost particle exchange via SPH's BasicSPHGhostHandler - Neighbor search using compressed BVH trees - Timestep calculation based on CFL and signal velocity modules/io/VTKDump.hpp/cpp: - VTK Legacy format output for visualization - Outputs positions, velocity, density, pressure, internal energy - Compatible with ParaView SPH code reuse (no duplication): - IterateSmoothingLengthDensity: uses SPH's module for h-iteration - LoopSmoothingLengthIter: uses SPH's outer loop - Ghost handling: BasicSPHGhostHandler for all BC types - Forces: sph_pressure_symetric() with Riemann solver pressure - Density utilities: rho_h(), h_rho(), newtown_iterate_new_h() - Safe division: inv_sat_zero() for numerical stability Performance improvement: - Density/omega computed ONCE after h converges (not on every iteration) - Previous approach recomputed density/omega 10+ times wastefully Fixes compatibility with main: - Use get_eos_gamma() instead of gamma member variable - Stub checkpoint methods (GSPHCheckpoint to be added separately)
- Add Model.hpp/cpp with high-level GSPH interface - Add pyGSPHModel.cpp with pybind11 bindings for Python - Use shared ShamrockDump mechanism for checkpoints (same as SPH) - Remove duplicate GSPHCheckpoint (reuse SPH pattern) - Add example scripts for Sod shock tube and Sedov blast - Add analytical Riemann solver utilities for validation - Clean up ad-hoc comments in Model.cpp
Guo-astro
force-pushed
the
gsph-bindings
branch
from
e03bde1 to
72f492f
Compare
Contributor
Author
|
Merged into PR #1453 for a cleaner single PR with all GSPH changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR completes the GSPH module by adding the Model class (Python interface) and Python bindings.
Depends on: #1453 (Solver and I/O)
Files added
Model.hpp/cpp:
pyGSPHModel.cpp:
Usage example
CMakeLists.txt updates
Test plan