-
Notifications
You must be signed in to change notification settings - Fork 50
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
automatic mode rotation for bent waveguide #2028
Conversation
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.
Adding some initial comments, overall the structure makes sense. So is this working currently? We'll obviously need to add a bunch of tests too.
Thanks! I’ve validated the process locally with several cases involving differently oriented bends, and I’ll add those to the tests. This PR isn’t fully working yet, but it’s close. I’ll wrap up the code based on your feedback. |
1353ea5
to
c1fd5c1
Compare
e9eb835
to
00976a1
Compare
6e51b86
to
4e6dbc7
Compare
c5f79fc
to
2517468
Compare
b3921cd
to
c8c4aac
Compare
6294a4f
to
df06ea9
Compare
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.
Almost there. One other thing is that this will go in 2.8, not in develop. One way to switch is something like:
git checkout -b backup/mode_rotation
git checkout qiming/mode_rotation
git fetch
git reset --hard origin/pre/2.8
git cherry-pick 8a718b6^..df06ea9 # I selected the sha-s of the three commits in this branch as of their current state but these could change if you e.g. rebase
git push -f origin qiming/mode_rotation # and then update the PR base here to be pre/2.8
10008a2
to
af4b955
Compare
20bd1b2
to
6e28a28
Compare
This is very helpful, much appreciated. |
6e28a28
to
e0b986a
Compare
e0b986a
to
7999058
Compare
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.
This looks ready, thanks for the great effort!
Just one small comment here and on the backend before merging.
…adius and angle_theta are defined
7999058
to
6e02992
Compare
@momchil-flex This is a very initial draft and not ready for review yet, but could you take a quick look at the code structure to see if there are any major mistakes so I can address them early on? I added a field
bend_correction
in mode_spec, and the process for mode rotation is mostly indata_raw
inModeSolver
.