search

Severson-Group / eMach

Open Source Machine Modeling and Optimization Framework. X Mach Speed.
https://emach.readthedocs.io
BSD 3-Clause "New" or "Revised" License
18 stars 3 forks source link

Wchan29/Add critical speed analyzer #320

Closed wchan29 closed 9 months ago

wchan29 commented 11 months ago

Issue

Address issue https://github.com/Severson-Group/eMach/issues/298

Summary

This PR adds rotor critical speed analyzer to eMach codebase.

closes #298

wchan29 commented 10 months ago

@dmnewman3 I have added the figure as requested. Please re-review.

wchan29 commented 10 months ago

@elsevers @Anvar-Khamitov @npetersen2 This is ready for Level 2 review.

wchan29 commented 9 months ago

I have made changes as requested by @elsevers

  1. The big one: Figure 1 is a screenshot from a publication. And while it is very good that you cite the source (this avoids plagiarism), copy-pasting an image like this creates a copyright issue. I think we code this into our RST file natively (i.e., no screenshot, instead create a table using RST code; probably use Inkscape to make the images that go with it).

I was able to create a table in Inkscape similar to the one from the textbook, I simplified it so only the diagrams and constant values are in the table.

  1. Figure 1 is providing values of $\betan l$. It is not clear how these correspond to $\beta{fi}$ in the equation above it?

I modified the input for the problem class from beta_fi to beta_l so everything should be consistent.

  1. The table Input for rotor critical speed problem is missing $\beta_{fi}$.

Should be fixed now.

@dmnewman3 ready for Level 1 re-review.

wchan29 commented 9 months ago

Ready for 2nd level review again. @elsevers @npetersen2 @Anvar-Khamitov

wchan29 commented 9 months ago

Bumping this again, Ready for 2nd level review again. @elsevers @npetersen2 @Anvar-Khamitov

elsevers commented 9 months ago

@dmnewman3 , can you please approve this PR? (your comments indicate you approve, my request is just to make the green checkbox appear next to your name)

dmnewman3 commented 9 months ago

@dmnewman3 , can you please approve this PR? (your comments indicate you approve, my request is just to make the green checkbox appear next to your name)

All set @elsevers. All set to merge @wchan29.