Skip to content
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

Fix critical speeds in Campbell diagram #1133

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

jguarato
Copy link
Contributor

This PR addresses an issue in the Campbell diagram where critical speeds were erroneously appearing in regions where they should not be present, as illustrated in the figure below. Upon investigation, the problem was identified as stemming from difficulties in accurately tracking vibration modes across variations in rotor speed. The lack of a reliable mode identification mechanism led to inconsistencies in the diagram:

WhatsApp Image 2024-12-12 at 14 40 06

To resolve this issue, a mode identification criterion was implemented in run_campbell. The Modal Assurance Criterion (MAC) was selected for its simplicity to implement and effectiveness in tracking modes. In addition to implementing the MAC criterion, further adjustments were made to improve the overall functionality the algorithm.

With these changes, the Campbell diagram for the previously analyzed case, considering the same six frequencies, now displays the correct behavior, as shown in the updated figure below:

WhatsApp Image 2024-12-12 at 14 34 31

Below are additional test results, with the old diagrams shown on the left and the corrected diagrams on the right:

rotor = rs.Rotor.load("compressor_example.toml")
samples = 81
speed_range = np.linspace(315, 1150, samples)
campbell = rotor.run_campbell(speed_range, frequencies=6)
campbell.plot(harmonics=[1]).show()

out

rotor = rs.rotor_example()
speed = np.linspace(0, 500, 101)
camp = rotor.run_campbell(speed, frequencies=6)
camp.plot(
    harmonics=[1, 2],
    damping_parameter="damping_ratio",
    frequency_range=rs.Q_((2000, 10000), "RPM"),
    damping_range=(-0.1, 100),
    frequency_units="RPM",
).show()

out

This final case, run with frequencies=7, produced the same Campbell diagram in both versions of ROSS, as shown below. This confirms that, with the new tracking method, the modes are now displayed in the correct order.
newplot(5)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.68%. Comparing base (f1bba84) to head (39ec89f).
Report is 49 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1133      +/-   ##
==========================================
+ Coverage   81.67%   82.68%   +1.01%     
==========================================
  Files          38       38              
  Lines        8310     7993     -317     
==========================================
- Hits         6787     6609     -178     
+ Misses       1523     1384     -139     
Files with missing lines Coverage Δ
ross/results.py 78.99% <100.00%> (+0.37%) ⬆️
ross/rotor_assembly.py 94.26% <100.00%> (+0.17%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43b038c...39ec89f. Read the comment docs.

@jguarato jguarato force-pushed the fix/critical-speed branch 2 times, most recently from ef1dc95 to f271a31 Compare January 15, 2025 16:45
@raphaeltimbo raphaeltimbo merged commit 695e6fe into petrobras:main Jan 16, 2025
2 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants