First prototype of the drone interface - #2001
Conversation
|
@ndevelder , here is the first draft of the drone interface that we discussed. |
|
This PR is ready for review. The failed test is expected. This PR corrects the actuator-sector behavior, so the previous reference results are no longer valid and should be updated. |
| amrex::Real z{0.0_rt}; | ||
| }; | ||
|
|
||
| inline Quaternion normalized(Quaternion q) |
There was a problem hiding this comment.
I think we should just let the compiler do any inlining and not do it ourselves.
There was a problem hiding this comment.
I rearranged the code. That inline was needed because the function definition was in the header file, but I have adjusted the definitions of the quaternion.
d-montgomery
left a comment
There was a problem hiding this comment.
This is more of a general comment, but would it make sense to restructure the actuator module into a generic framework layer (src/actuator/) and separate domain-specific applications (src/applications/wind_energy/, src/applications/aerospace/)? This would make it clearer which components are reusable versus domain-specific, and would naturally place drone under aerospace rather than wind_energy. Something like this:
src/
├── actuator/
│ ├── disk/
│ ├── drone/
│ ├── motion/
│ ├── sector/
│ ├── turbine/
│ ├── wing/
│ ├── actuator_types.H
│ └── ...
├── applications/
│ └── aerospace/
│ └── drone/
│ ├── hydrokinetic/
│ ├── wind_energy/
└── ...
Not something that needs to be addressed here, but worth considering as we expand SGF into different application domains.
| Quaternion& Quaternion::normalize() | ||
| { | ||
| const amrex::Real norm = std::sqrt(w * w + x * x + y * y + z * z); | ||
| if (norm <= std::numeric_limits<amrex::Real>::epsilon()) { |
There was a problem hiding this comment.
Minor and nitpicky, but this is included in the constants namespace constants::EPS and is already used elsewhere in this file.
There was a problem hiding this comment.
Good catch, I have adjusted it and pushed the latest code. The tolerance epsilon was used in a few places and all are fixed now.
I agree with the comment regarding the code structure, but that is a decision that the code maintainers need to make. This PR will use the current structure of the code.
There was a problem hiding this comment.
Pull request overview
Adds a first-cut multi-rotor Drone actuator built from ActuatorSector rotors, and introduces a shared “prescribed motion” layer (rigid-body + rotor-speed histories) that can drive both standalone sectors and drone composites. This expands the actuator system with time-dependent translation/orientation and synchronized multi-rotor behavior, plus corresponding tests and user docs.
Changes:
- Introduces prescribed-motion infrastructure (
TimeTable,RigidBodyMotion,RotorMotion) and wires it intoActuatorSector. - Adds the
Droneactuator type as a composite of sector rotors, including NetCDF output as nested groups. - Adds unit/regression tests and Sphinx documentation for new inputs and capabilities.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| unit_tests/wind_energy/actuator/test_drone.cpp | Unit tests for drone geometry, lifecycle, equivalence to standalone sectors, and NetCDF grouping. |
| unit_tests/wind_energy/actuator/test_actuator_motion.cpp | Unit tests for timetable interpolation/integration and rigid-body/rotor motion histories. |
| unit_tests/wind_energy/actuator/CMakeLists.txt | Adds new actuator unit tests to the build. |
| test/test_files/act_drone_quad/act_drone_quad.inp | New regression-test input file for a quadcopter drone actuator. |
| test/CMakeLists.txt | Registers the new act_drone_quad regression test. |
| src/wind_energy/actuator/sector/ActuatorSector.H | Extends sector metadata with shared motion objects and integrated force/moment diagnostics. |
| src/wind_energy/actuator/sector/actuator_sector_ops.H | Adds rotor placement helper and NetCDF group-based APIs. |
| src/wind_energy/actuator/sector/actuator_sector_ops.cpp | Implements motion-driven placement/updates, NetCDF group writes, and integrated force/moment accumulation. |
| src/wind_energy/actuator/motion/TimeTable.H | New piecewise-linear multi-component timetable interface. |
| src/wind_energy/actuator/motion/TimeTable.cpp | Timetable parsing plus consistent value/derivative/integral evaluation. |
| src/wind_energy/actuator/motion/RotorMotion.H | New rotor-speed history support with azimuth integration. |
| src/wind_energy/actuator/motion/RotorMotion.cpp | Parses drone/sector rotor-speed sources and computes omega/azimuth. |
| src/wind_energy/actuator/motion/RigidBodyMotion.H | New rigid-body translation/orientation history interface. |
| src/wind_energy/actuator/motion/RigidBodyMotion.cpp | Implements exclusive motion-source parsing and quaternion-based orientation interpolation. |
| src/wind_energy/actuator/motion/CMakeLists.txt | Builds the new motion sources into the library. |
| src/wind_energy/actuator/drone/Drone.H | Declares the Drone actuator type and rotor layout helpers. |
| src/wind_energy/actuator/drone/Drone.cpp | Implements drone layout helpers and registers the actuator model. |
| src/wind_energy/actuator/drone/drone_ops.H | Declares ops specializations for reading, updating, forcing, and outputs. |
| src/wind_energy/actuator/drone/drone_ops.cpp | Implements drone composition, shared motion injection into rotors, and nested-group NetCDF output. |
| src/wind_energy/actuator/drone/CMakeLists.txt | Builds the new drone actuator sources into the library. |
| src/wind_energy/actuator/CMakeLists.txt | Adds motion/ and drone/ subdirectories to the actuator build. |
| src/wind_energy/actuator/ActuatorModel.H | Adds grid() accessor for diagnostics/inspection. |
| src/wind_energy/actuator/actuator_types.H | Adds ActSrcDrone source trait for the composite drone actuator. |
| src/core/vs/tensorI.H | Removes the inlined axis-angle quaternion-to-tensor helper from this header. |
| src/core/vs/tensor.H | Includes quaternion support to preserve existing include behavior. |
| src/core/vs/quaternion.H | New quaternion type and utilities (tensor conversion, slerp, etc.). |
| src/core/vs/quaternion.cpp | Implements quaternion operations and slerp. |
| src/core/CMakeLists.txt | Adds quaternion implementation file to the core library build. |
| docs/sphinx/user/inputs_Actuator.rst | Documents prescribed-motion inputs and the new Drone actuator. |
| docs/sphinx/user/features.rst | Lists prescribed-motion rotor/drone capability in features. |
| docs/sphinx/spelling-wordlist.txt | Adds new technical terms for doc spelling checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| amrex::Real cosine = dot(a, b); | ||
| // At exactly 180 degrees the interpolation path is not unique. Ignore a | ||
| // roundoff-sized negative dot product so equivalent RPY and quaternion | ||
| // inputs choose the same path. | ||
| if (cosine < -constants::EPS) { | ||
| b = {-b.w, -b.x, -b.y, -b.z}; | ||
| cosine = -cosine; | ||
| } else if (cosine < 0.0_rt) { | ||
| cosine = 0.0_rt; | ||
| } |
There was a problem hiding this comment.
This has been fixed. There was an issue with the interpolation when there is 180deg separation between angles. The code will now give a warning when that is the case.
|
This produces diffs on the regression tests (act_sector, act_sector_quad) that were added in your last commit. Are those diffs expected? |
Yes, we made some changes in this PR that resulted in changes to the gold files. There were some changes in quaternion handling that leads to small differences and another bug fix in the actuator implementation. This is all correct as of this latest version and the gold files for those 2 tests need to be updated. |
|
Sounds great. I'll skim over the code, and, unless I find something, it should be good to go. After it's approved, could you update it with the latest changes to main? Then it'll be ready to get merged. We might to a follow-on PR with the restructuring that Dave proposed, so heads up for that. Though I don't know how that could affect your other PR in draft. |
Co-authored-by: Michael B Kuhn <31661049+mbkuhn@users.noreply.github.com>
|
Yeah, I think that would be fine! Could you update this branch? (Eliminate the "This branch is out-of-date..." note) |
perfect, thanks! I have updated the branch. |
Summary
Adding a new interface for drones to combine propellers.
Please check the type of change introduced:
Checklist
The following is included:
This PR was tested by running:
Additional background
Issue Number: