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

BugFix: refpts=None in get_lifetime #580

Merged
merged 5 commits into from
Mar 14, 2023
Merged

BugFix: refpts=None in get_lifetime #580

merged 5 commits into from
Mar 14, 2023

Conversation

swhite2401
Copy link
Contributor

PR #577 introduced a bug when refpts=None, this PR fixes this issue

@swhite2401
Copy link
Contributor Author

@oscarxblanco sorry I overlooked the case where refpts=None in the previous PR, this one should fix the issue.

Copy link
Contributor

@oscarxblanco oscarxblanco left a comment

Choose a reason for hiding this comment

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

I have checked the call to get_uint32_index(at.All, endpoint=False) and it seems to work as expected for a given ring with 4043 elements

>>> len(ring)
4043
>>> ring.get_uint32_index(at.All, endpoint=False)
array([   0,    1,    2, ..., 4040, 4041, 4042], dtype=uint32)
>>> ring.get_uint32_index(at.All)
array([   0,    1,    2, ..., 4041, 4042, 4043], dtype=uint32)

When executing a test example, both options, with or w/o refpts work and write the expected warning message in the standard output refering to zero-length elements in the index list. This is normal behavior.

I was not able to finish the whole momentum aperture calculation due to some segmentation fault. But this seems to be not related to the indexing problem.

@swhite2401
Copy link
Contributor Author

Ok so I propose to merge this as it fixes an obvious bug, on the other hand it would be good to understand this segmentation fault, I you have more details could you report it as a separate issue (it is most likely originating from the c part of the code).

I usually never run LT calculation for the full lattice but rather on a subset of elements (typically magnets) so it is well possible that for very heavy computations some problems have passed through the various validations.

@oscarxblanco
Copy link
Contributor

@swhite2401 ok, I will report the segmentation fault in another issue.

From my side, this fix could be merged.

@swhite2401 swhite2401 merged commit d1fbfca into master Mar 14, 2023
@swhite2401 swhite2401 deleted the get_lifetime_refpts branch March 14, 2023 16:06
@lfarv lfarv added Python For python AT code bug fix labels Mar 29, 2023
@lfarv lfarv mentioned this pull request Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Python For python AT code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants