Skip to content

[SPH][Setups] New custom warp setup#1162

Merged
tdavidcl merged 16 commits intoShamrock-code:mainfrom
y-lapeyre:feature/complex_warp
Aug 20, 2025
Merged

[SPH][Setups] New custom warp setup#1162
tdavidcl merged 16 commits intoShamrock-code:mainfrom
y-lapeyre:feature/complex_warp

Conversation

@y-lapeyre
Copy link
Copy Markdown
Collaborator

Allows users to define a custom warp function from the python interface.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @y-lapeyre, 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!

I've implemented a new feature that allows users to define and apply custom warp transformations to the SPH simulation setup. This enhancement provides greater flexibility by enabling the use of arbitrary functions for warp profiles, which can be specified directly from the Python interface. The change involves adding a new C++ module to handle the custom warping logic, integrating it into the existing setup framework, and exposing it through the Python bindings.

Highlights

  • New Custom Warp Module: I've introduced a new C++ module, ModifierApplyCustomWarp, which provides the core functionality for applying user-defined warp transformations to simulation particles. This module is designed to be flexible, accepting custom profile functions for inclination, psi, and k-vector.
  • Integration into SPHSetup: I've integrated the new custom warp functionality into the existing SPHSetup class by adding a make_modifier_custom_warp method. This method facilitates the creation and configuration of the custom warp modifier within the SPH setup pipeline.
  • Python Interface for Custom Warps: I've exposed the make_modifier_custom_warp function to the Python interface. This allows Python users to easily define and apply complex, custom warp functions directly from their Python scripts, enhancing the flexibility and usability of the SPH model.
  • Build System Update: I've updated the CMake build configuration to include the newly added C++ source file for the custom warp modifier, ensuring it is properly compiled and linked with the rest of the project.
Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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 or fill out our survey to provide feedback.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 new custom warp modifier, which is a great feature. The implementation is mostly in place, but there are several areas that need attention before this can be merged. Most importantly, the psi_profile parameter is accepted by the new modifier but is not actually used in the warp calculation, which appears to be a bug. Additionally, the code contains leftover debugging logs, some unused variables, and opportunities for minor improvements in C++ best practices and Python API consistency. Addressing these points will improve the code's correctness, cleanliness, and maintainability.

Comment thread src/shammodels/sph/src/modules/setup/ModifierApplyCustomWarp.cpp Outdated
Comment thread src/shammodels/sph/src/modules/setup/ModifierApplyCustomWarp.cpp Outdated
Comment thread src/shammodels/sph/src/modules/SPHSetup.cpp
Comment thread src/shammodels/sph/src/modules/setup/ModifierApplyCustomWarp.cpp Outdated
Comment thread src/shammodels/sph/src/pySPHModel.cpp Outdated
@tdavidcl tdavidcl added the draft label Aug 8, 2025
@y-lapeyre y-lapeyre marked this pull request as ready for review August 11, 2025 13:59
@y-lapeyre
Copy link
Copy Markdown
Collaborator Author

Computation is done on the host since passing a function to a cycle kernel is not easy. From browsing the internet it seems possible to do through a clever use of functors, so might try that later. This module was tested on some usual warp setups.

Comment thread doc/sphinx/examples/sph/run_sph_custom_warp_profile.py Outdated
@tdavidcl tdavidcl added in-review and removed draft labels Aug 12, 2025
@github-actions
Copy link
Copy Markdown
Contributor

Workflow report

workflow report corresponding to commit e0a8f7a
Commiter email is yona.lapeyre@ens-lyon.fr
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
black....................................................................Passed
ruff check...............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed

Test pipeline can run.

Clang-tidy diff report

No relevant changes found.
Well done!

You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review.

Doxygen diff with main

Removed warnings : 5
New warnings : 8
Warnings count : 7450 → 7453 (0.0%)

Detailed changes :
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:73: warning: Member make_modifier_custom_warp(SetupNodePtr parent, std::function< Tscal(Tscal)> inc_profile, std::function< Tscal(Tscal)> psi_profile, std::function< Tvec(Tscal)> k_profile) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:74: warning: Member make_modifier_add_offset(SetupNodePtr parent, Tvec offset_postion, Tvec offset_velocity) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:77: warning: Member make_modifier_filter(SetupNodePtr parent, std::function< bool(Tvec)> filter) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:80: warning: Member make_modifier_add_offset(SetupNodePtr parent, Tvec offset_postion, Tvec offset_velocity) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:83: warning: Member make_modifier_filter(SetupNodePtr parent, std::function< bool(Tvec)> filter) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/setup/ModifierApplyCustomWarp.hpp:26: warning: Compound shammodels::sph::modules::ModifierApplyCustomWarp is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/setup/ModifierApplyCustomWarp.hpp:40: warning: Member ModifierApplyCustomWarp(ShamrockCtx &context, Config &solver_config, SetupNodePtr parent, std::function< Tscal(Tscal)> inc_profile, std::function< Tscal(Tscal)> psi_profile, std::function< Tvec(Tscal)> k_profile) (function) of class shammodels::sph::modules::ModifierApplyCustomWarp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:782: warning: Member add_analysisBarycenter_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:796: warning: Member add_analysisBarycenter_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:801: warning: Member analysisBarycenter_impl(shammodels::sph::Model< Tvec, SPHKernel > &model) -> modules::AnalysisBarycenter< Tvec, SPHKernel > (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:806: warning: Member Register_pymod(pysphmodel) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:815: warning: Member analysisBarycenter_impl(shammodels::sph::Model< Tvec, SPHKernel > &model) -> modules::AnalysisBarycenter< Tvec, SPHKernel > (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:820: warning: Member Register_pymod(pysphmodel) (function) of file pySPHModel.cpp is not documented.

@tdavidcl tdavidcl merged commit 4aecd18 into Shamrock-code:main Aug 20, 2025
92 of 95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants