[SPH][Setups] New custom warp setup#1162
Conversation
There was a problem hiding this comment.
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
-
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. ↩
There was a problem hiding this comment.
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.
|
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. |
…into feature/complex_warp
…into feature/complex_warp
Workflow reportworkflow report corresponding to commit e0a8f7a Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportNo relevant changes found. You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review. Doxygen diff with
|
Allows users to define a custom warp function from the python interface.