-
Notifications
You must be signed in to change notification settings - Fork 18
[GSPH 2/7] Newtonian physics mode #1524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GSPH 2/7] Newtonian physics mode #1524
Conversation
|
Thanks @Guo-astro for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
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 introduces a comprehensive Newtonian physics mode for the GSPH (Godunov Smoothed Particle Hydrodynamics) simulation framework. It establishes a robust foundation for simulating non-relativistic hydrodynamics, featuring a modular design that allows for configurable equations of state, advanced Riemann solvers for inter-particle force calculations, and flexible spatial reconstruction techniques. The new mode integrates a kick-drift-kick (KDK) leapfrog timestepping scheme, ensuring accurate and stable evolution of particle states. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the complete Newtonian physics mode for GSPH, including configurations for the solver, equation of state, field names, force kernels, and timesteppers. It also adds support for different Riemann solvers and reconstruction methods. The changes are extensive and well-structured, providing a solid foundation for Newtonian hydrodynamics within the SHAMROCK framework. All provided comments offer valid and useful feedback, addressing areas where consistency, maintainability, and completeness could be improved, particularly regarding the dynamic selection of Riemann solvers and the use of named constants for field identifiers.
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/NewtonianForceKernel.hpp
Show resolved
Hide resolved
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/NewtonianMode.hpp
Show resolved
Hide resolved
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/ReconstructConfig.hpp
Show resolved
Hide resolved
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/ReconstructConfig.hpp
Show resolved
Hide resolved
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/RiemannConfig.hpp
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive Newtonian physics mode for GSPH, including configurations, timestepping, EOS, and force computations with various Riemann solvers. The overall structure is well-designed, using modern C++ features like std::variant for flexible configuration. However, there are several areas for improvement. The implementation currently hardcodes the choice of Riemann solver and reconstruction method, bypassing the flexible configuration system that was also introduced. There's also significant code duplication in the force kernels and a minor inconsistency in the EOS implementation. Additionally, a performance improvement opportunity exists in the force calculation loop. Addressing these points will make the new module more robust, maintainable, and aligned with the configurable design.
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/forces.hpp
Show resolved
Hide resolved
src/shammodels/gsph/include/shammodels/gsph/physics/newtonian/riemann/Iterative.hpp
Show resolved
Hide resolved
45e892a to
e51d1f4
Compare
Core Infrastructure: - PhysicsMode base class for strategy pattern implementation - ForceKernelBase for common force computation interface - PhysicsModeFactory for creating physics mode instances - FieldNames.hpp as SSOT for field naming Modular Components: - BoundaryHandler: boundary condition processing - BuildTrees: tree construction - ComputeCFL: CFL timestep calculation - ComputeGradients: gradient computation - ComputeOmega: omega factor computation - GhostCommunicator: MPI ghost communication - IterateSmoothingLengthVolume: h-iteration - NeighbourCache: caching neighbor interactions - FunctorNode: generic functor nodes Refactors the monolithic GSPH Solver into modular components, preparing for physics mode decoupling (Newtonian vs SR).
The ForceKernelBase template class was designed as a Template Method pattern base but was never used - NewtonianForceKernel and SRForceKernel are standalone implementations with their own buffer management appropriate for their physics. The hardcoded CommonBuffers design (buf_density, buf_pressure, etc.) does not accommodate SR physics which requires distinct lab-frame vs rest-frame field naming (N_LABFRAME, LORENTZ_FACTOR, ENTHALPY, etc.).
These fields have physics-specific meanings: - Newtonian: single frame quantities - SR: lab-frame vs rest-frame distinction Each physics mode now defines its own field constants with appropriate semantic names in their respective FieldNames headers.
Headers: - NewtonianConfig: solver configuration for Newtonian hydro - NewtonianEOS: ideal gas equation of state interface - NewtonianFieldNames: field naming constants - NewtonianForceKernel: force computation interface - NewtonianMode: PhysicsMode implementation for Newtonian hydro - NewtonianTimestepper: CFL-based timestepping - forces.hpp: force math functions - ReconstructConfig: interface reconstruction settings - RiemannConfig: Riemann solver configuration Riemann Solvers: - RiemannBase: abstract Riemann solver interface - HLL: Harten-Lax-van Leer approximate Riemann solver - Iterative: exact iterative Riemann solver Sources: - Complete implementations for all Newtonian components This implements the Newtonian physics mode for GSPH using the strategy pattern for physics decoupling.
Energy fields have physics-specific meanings and are now defined directly in NewtonianFieldNames.hpp rather than imported from the common FieldNames.hpp.
Headers: - SRConfig: solver configuration for SR hydro - SREOS: relativistic equation of state interface - SRFieldNames: field naming constants for SR fields - SRForceKernel: force computation interface for SR - SRMode: PhysicsMode implementation for SR hydro - SRPrimitiveRecovery: conservative to primitive variable recovery - SRTimestepper: relativistic CFL-based timestepping - forces.hpp: SR force math functions Sources: - SREOS.cpp: relativistic EOS implementation - SRForceKernel.cpp: SR force kernel implementation - SRPrimitiveRecovery.cpp: Newton-Raphson primitive recovery - SRTimestepper.cpp: SR timestep computation This implements the core SR physics components for GSPH.
SR physics defines its own XYZ, VXYZ, AXYZ, UINT, DUINT constants directly rather than importing from the common FieldNames.hpp. This clarifies the physical meaning (lab-frame quantities for SR).
Recovery Methods: - RecoveryBase: abstract interface for primitive recovery - NewtonRaphson: Newton-Raphson iterative recovery algorithm Riemann Solvers: - RiemannBase: abstract SR Riemann solver interface - Exact: exact relativistic Riemann solver (Pons 2000) Mode Implementation: - SRMode.cpp: complete SR physics mode orchestration - Ghost field setup for SR variables - Primitive recovery from conserved variables - Force computation with relativistic corrections - Integration of SR equations This completes the Special Relativistic GSPH implementation following Kitajima 2024 formulation.
Solver Changes: - Solver.hpp/cpp: Refactored to use PhysicsMode strategy pattern - SolverConfig.hpp/cpp: Updated config for physics mode selection - Model.hpp/cpp: Updated model registration Storage & IO: - SolverStorage.hpp: Updated field storage for physics modes - VTKDump.hpp/cpp: Physics-aware VTK output Python Bindings: - pyGSPHModel.cpp: Extended bindings for SR configuration - physics_mode selection (newtonian/sr) - SR-specific parameters (gamma, initial conditions) Build System: - CMakeLists.txt: Updated for new physics module structure This integrates the modular physics modes into the GSPH solver, enabling runtime selection between Newtonian and SR physics.
Newtonian Tests: - sod_tube_gsph.py: Sod shock tube validation - blast_wave_gsph.py: Extreme blast wave test SR Tests (Kitajima 2024 benchmark suite): - problem1_sod.py: Relativistic Sod shock tube - problem2_blast.py: Relativistic blast wave - problem3_strong_blast.py: Strong relativistic blast - problem4_ultra_relativistic.py: Ultra-relativistic regime - problem5_tangent_velocity.py: Tangential velocity test - problem6_2d_sod.py: 2D relativistic Sod tube - problem7_kh_instability.py: Kelvin-Helmholtz instability Common: - sr/__init__.py: SR test utilities - kitajima_plotting.py: Plotting helpers for Kitajima benchmarks All tests use: - ctx.collect_data() for direct memory access (no pyvista) - Strict tolerances (~1e-8) for regression testing - Analytic solutions for validation
Unit Tests: - GSPHForceTests.cpp: Update for new physics structure - GSPHRiemannTests.cpp: Update Riemann solver tests SPH Module Fixes: - IterateSmoothingLengthDensity: Improve h-iteration logging - BasicSPHGhosts.cpp: Fix ghost handling Math: - sphkernels.hpp: Minor kernel fixes MHD Placeholder: - MHDConfig.hpp: Placeholder for future MHD physics mode
- NewtonianMode: add compute_omega_newtonian() using standard SPH (no c_smooth) - SRMode: use SRIterateSmoothingLength with Kitajima volume-based approach - Remove shared ComputeOmega module (each mode now owns this) - Remove legacy UpdateDerivs (replaced by NewtonianForceKernel) - Move IterateSmoothingLengthVolume to physics/sr/SRIterateSmoothingLength - Update CMakeLists.txt sources This fixes the density/pressure error regression caused by c_smooth=1.2 being incorrectly applied to Newtonian mode.
…ult (1.0) SR's volume-based h-iteration (Kitajima Eq. 232-233) requires c_smooth > 1 to smooth h variation across discontinuities. The SR-specific value was defined in SRConfig but not transferred to the shared config.
The Riemann solver and force computation code has been moved to the physics/newtonian/ and physics/sr/ directories. The math/ folder contained duplicate/orphaned code that is no longer used.
Remove deprecated include of math/riemann/iterative.hpp (now deleted) and update hllc_solver call to use solve_hll from the new location in physics/newtonian/riemann/HLL.hpp.
c95ae36 to
26ad744
Compare
Run buildbot/update_authors.py to add --no git blame-- annotations to author headers as required by CI checks.
Remove SolverCallbacks struct and have PhysicsMode evaluate nodes directly via storage.solver_graph. This aligns with solvergraph design: - Branching happens at init time (node registration) - Flow is visible as graph structure - No runtime callback creation Changes: - Register Solver method nodes in init_solver_graph() - Delete SolverCallbacks struct from PhysicsMode.hpp - Update evolve_timestep() signature (remove callbacks param) - NewtonianMode/SRMode evaluate nodes directly via storage.solver_graph Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Adds the complete Newtonian physics mode for GSPH.
Headers
NewtonianConfig: solver configuration for Newtonian hydroNewtonianEOS: ideal gas equation of state interfaceNewtonianFieldNames: field naming constantsNewtonianForceKernel: force computation interfaceNewtonianMode: PhysicsMode implementationNewtonianTimestepper: CFL-based timesteppingforces.hpp: force math functionsReconstructConfig: interface reconstruction settingsRiemannConfig: Riemann solver configurationRiemann Solvers
RiemannBase: abstract Riemann solver interfaceHLL: Harten-Lax-van Leer approximate Riemann solverIterative: exact iterative Riemann solverSources
Complete implementations for all Newtonian components.
Dependencies
Depends on #1523 (core infrastructure)