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 empty orbit in atlinopt #434

Merged
merged 4 commits into from
Jul 5, 2022
Merged

fix empty orbit in atlinopt #434

merged 4 commits into from
Jul 5, 2022

Conversation

swhite2401
Copy link
Contributor

@swhite2401 swhite2401 commented Jul 4, 2022

This PR fixes an issue with atlinopt with twiss_in activated.
When orbit_in was not provided an empty list was returned instead of 6 zeros.
The implementation could be further improved by using twiss_in.ClosedOrbit instead of zeros(6,1), to be discussed
If present the default is twiss_in.ClosedOrbit

@swhite2401 swhite2401 changed the title fix empty orbit in fix empty orbit in in atlinopt Jul 4, 2022
@lfarv
Copy link
Contributor

lfarv commented Jul 4, 2022

The implementation could be further improved by using twiss_in.ClosedOrbit instead of zeros(6,1)

This is what is done in atlinopt6, I just copied the 4 lines !

Copy link
Contributor

@lfarv lfarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@lfarv lfarv added Matlab For Matlab/Octave AT code bug fix labels Jul 5, 2022
@swhite2401 swhite2401 changed the title fix empty orbit in in atlinopt fix empty orbit in atlinopt Jul 5, 2022
@swhite2401
Copy link
Contributor Author

Thanks, I checked and this is what is done in pyAT as well so I think we are all good

@lfarv
Copy link
Contributor

lfarv commented Jul 5, 2022

@swhite2401 : I think this is not the right place for the fix. atlinopt6 is designed to accept twiss input in any format, so what happens here is a bug in atlinopt6, not atplot. Let me have a look.

@swhite2401
Copy link
Contributor Author

Well if the rmatrix is missing it will fail... the other option is to provide a function that derives the rmatrix from atlinopt output but do you really want to go in this direction?

@swhite2401
Copy link
Contributor Author

By the way the same problem is in pyAT

@lfarv
Copy link
Contributor

lfarv commented Jul 5, 2022

Of course yes: if R is missing, atlinopt6 (build_sigma) takes alpha and beta from the twiss input. But something must be missing… atlinopt6 has not been tested enough in transfer line mode, thanks @simoneliuzzo. Could you separate the two problems, rollback to commit aa1fd9e and open another PR for atplot?

@lfarv
Copy link
Contributor

lfarv commented Jul 5, 2022

By the way the same problem is in pyAT

Another good reason to open a dedicated PR !

@swhite2401
Copy link
Contributor Author

Ok I roll back the last commit and merge this one.

@swhite2401 swhite2401 merged commit edaab05 into master Jul 5, 2022
@swhite2401 swhite2401 deleted the atlinopt_twiss_in branch July 5, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Matlab For Matlab/Octave AT code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants