pyat get_lifetime modifies the given refpts

[oscarxblanco]

Dear all,

the function get_lifetime has a parameter to pass the reference points, refpts, where the momentum aperture momap was calculated. However, in line 146 of pyat/at/acceptance/touschek.py this array is modified leading to a mismatch between momap and refpts.

https://github.com/atcollab/at/blob/fee2a6a462ad263aa722b2c8b0ffa3dafdce8332/pyat/at/acceptance/touschek.py#L142-L147

I think that refpts should not be modified when given by the user. One could think to use a marker as a reference point which should not be removed from the list even if it has zero length.

Best regards, o

[oscarxblanco]

One possible solution is to execute those lines only when refpts is not passed, i.e. move them inside the if refpts is None case.

[oscarxblanco]

One other posibility is that due to the implementation of get_lifetime, one needs to pass only no-zero length elements.

If that is the case the function should check the elements length and send a warning/error if any is of length zero. This could be done by comparing the length of the filtered list to the original one, or by checking that the list of elements with zero length is void.

[swhite2401]

Hello @oscarxblanco, Thanks for the feedback. In fact this is not fully satisfactory should be improved. The zero-length elements are indeed causing problems. This is why the internal refpts is returned.

What I can propose in the short term based on your remarks is: -if refpts is None no change -if refpts is provided check that all corresponding elements have non-zero length and throw a warning if this condition is not respected

Then we may think of trying to improve the algorithm to account for zero length elements (length could be in this case the distance to the next refpts but this could be dangerous if this distance is very large...)

Let me know in case you have better proposals. This was basically translated from the matlab and there is certainly room for improvement.

[swhite2401]

By the way in case you are interested strictly in momentum aperture you can run get_momentum_acceptance. This will not interfere with your refpts input. Then you can give the result of this calculation together with refpts to get_lifetime.

Removing the refpts in get_lifetime that will not be used (zero length elements) is also ment to optimize the computer time.

[oscarxblanco]

Hello @oscarxblanco, Thanks for the feedback. In fact this is not fully satisfactory should be improved. The zero-length elements are indeed causing problems. This is why the internal refpts is returned.

What I can propose in the short term based on your remarks is: -if refpts is None no change -if refpts is provided check that all corresponding elements have non-zero length and throw a warning if this condition is not respected

Then we may think of trying to improve the algorithm to account for zero length elements (length could be in this case the distance to the next refpts but this could be dangerous if this distance is very large...)

Let me know in case you have better proposals. This was basically translated from the matlab and there is certainly room for improvement.

Thank you, it sounds alright, except for the warning. In my opinion it should throw an error as the algorithm does not yet support zero-length elements.

By the way in case you are interested strictly in momentum aperture you can run get_momentum_acceptance. This will not interfere with your refpts input. Then you can give the result of this calculation together with refpts to get_lifetime.

Removing the refpts in get_lifetime that will not be used (zero length elements) is also ment to optimize the computer time.

Thank you. This is exactly how I found out this behaviour. I calculated the momentum acceptance using a list of unique s coordinates without checking the element's length. When passing the indexes and the momentum aperture, the get_lifetime function always show a misleading error saying that the two inputs were of different length, when they weren't. One of my inputs was modified inside the routine.

[swhite2401]

Ah ok then this is a more severe bug: in case refpts and momentum aperture are provided, zero length elements should be removed form both.

[oscarxblanco]

Ah ok then this is a more severe bug: in case refpts and momentum aperture are provided, zero length elements should be removed form both.

Yes, that should be ok if the used indexes are returned. I would suggest a verbose output option to let the user know if indexes were modified by this filtering check.

[swhite2401]

Fixed in #577