-
Notifications
You must be signed in to change notification settings - Fork 136
docs: adding a PSD example #4115
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
Conversation
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
Reviewer's GuideThis PR implements a new rpsd wrapper in the gRPC client to return response power spectral density results as numpy arrays, adds unit tests and a fixture for PSD analysis, and contributes a complete PSD example script (VM203) for the documentation. Class diagram for the new rpsd method in MapdlGrpcclassDiagram
class MapdlBase {
+rpsd(ir, ia, ib, itype, datum, name, signif, **kwargs)
}
class MapdlGrpc {
+rpsd(ir, ia, ib, itype, datum, name, signif, **kwargs) NDArray[np.float64]
}
MapdlGrpc --|> MapdlBase
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Pull Request Overview
This PR adds a Power Spectral Density (PSD) analysis example to the documentation, demonstrating random vibration analysis using PyMAPDL with ANSYS Verification Manual problem VM203.
- Adds a comprehensive PSD analysis example with modal analysis, spectrum analysis, and post-processing
- Implements wrapper function for the RPSD command to return numpy arrays
- Includes test coverage for the new RPSD functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
examples/00-mapdl-examples/psd-vm203.py | New comprehensive example demonstrating PSD analysis workflow |
src/ansys/mapdl/core/mapdl_grpc.py | Adds rpsd method wrapper to return numpy arrays |
tests/conftest.py | Adds psd_analysis fixture using VM203 file |
tests/test_mapdl.py | Adds test for rpsd command functionality |
Comments suppressed due to low confidence (1)
tests/conftest.py:1338
- The fixture runs VM203 input but doesn't ensure the analysis completes successfully or that the required data for PSD analysis is available. Consider adding validation or error handling to ensure the fixture provides a valid state for PSD testing.
mapdl.input(vmfiles["vm203"])
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.
Hey @germa89 - I've reviewed your changes - here's some feedback:
- The new rpsd wrapper uses vget('_temp', ir) where ir is the input index rather than a variable name—consider clarifying or standardizing the parameter so vget always retrieves the intended result array.
- The long example script executes on import; wrap the main execution block in an if name == 'main' guard to prevent unintended side effects when loading the module.
- The psd_analysis fixture only inputs the example file without running the necessary modal/PSD steps, so tests could be flaky—add the commands to complete the PSD setup in the fixture to guarantee a ready state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new rpsd wrapper uses vget('_temp', ir) where ir is the input index rather than a variable name—consider clarifying or standardizing the parameter so vget always retrieves the intended result array.
- The long example script executes on import; wrap the main execution block in an if __name__ == '__main__' guard to prevent unintended side effects when loading the module.
- The psd_analysis fixture only inputs the example file without running the necessary modal/PSD steps, so tests could be flaky—add the commands to complete the PSD setup in the fixture to guarantee a ready state.
## Individual Comments
### Comment 1
<location> `src/ansys/mapdl/core/mapdl_grpc.py:3316` </location>
<code_context>
+ signif=signif,
+ **kwargs,
+ )
+ return self.vget("_temp", ir)
+
def get_nsol(
</code_context>
<issue_to_address>
Potential mismatch in vget argument for rpsd return value.
Please confirm that 'ir' is the intended argument for vget here, as using the wrong variable index could return incorrect data.
</issue_to_address>
### Comment 2
<location> `examples/00-mapdl-examples/psd-vm203.py:259` </location>
<code_context>
+
+###############################################################################
+# plot the response psd
+freqs = mapdl.vget("FREQS", 1)
+
+# Remove the last two values as they are zero
</code_context>
<issue_to_address>
Potential for off-by-one error when slicing frequency and RPSD arrays.
Verify that removing the last two elements from freqs and rpsduz is always valid, as changes in the data structure could cause array length mismatches or omit important data.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# Remove the last two values as they are zero
plt.plot(freqs[:-2], rpsduz[:-2], label="RPSD UZ")
=======
# Remove the last two values as they are zero, but ensure array lengths match and are valid
if len(freqs) >= 2 and len(rpsduz) >= 2:
freqs_trimmed = freqs[:-2]
rpsduz_trimmed = rpsduz[:-2]
else:
raise ValueError("Frequency and RPSD arrays must have at least two elements to trim.")
if len(freqs_trimmed) != len(rpsduz_trimmed):
raise ValueError(
f"Length mismatch after trimming: freqs ({len(freqs_trimmed)}), rpsduz ({len(rpsduz_trimmed)})"
)
plt.plot(freqs_trimmed, rpsduz_trimmed, label="RPSD UZ")
>>>>>>> REPLACE
</suggested_fix>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4115 +/- ##
=======================================
Coverage 91.30% 91.30%
=======================================
Files 189 189
Lines 15649 15650 +1
=======================================
+ Hits 14289 14290 +1
Misses 1360 1360 🚀 New features to boost your workflow:
|
… in documentation build workflow
@pyansys-ci-bot LGTM. |
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.
Description
As the title. Add a PSD analysis to the docs.
Issue linked
Close #3865
Checklist
draft
if it is not ready to be reviewed yet.feat: adding new MAPDL command
)Summary by Sourcery
Add support for response power spectral density retrieval in the gRPC interface, include corresponding tests and fixtures, and provide a new PSD example script based on VM203.
New Features:
Documentation:
Tests: