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

refpts offset for acceptance #639

Merged
merged 1 commit into from
Aug 3, 2023
Merged

refpts offset for acceptance #639

merged 1 commit into from
Aug 3, 2023

Conversation

swhite2401
Copy link
Contributor

This PR answers #637. Individual offset can now be input for each refpts in get_acceptance in the form of an ndarray.

@swhite2401
Copy link
Contributor Author

@oscarxblanco, everything seems to work fine on my side could you please let me know if this is what you were looking for?

@oscarxblanco
Copy link
Contributor

oscarxblanco commented Aug 3, 2023

@swhite2401 I have made several tests to obtain the momentum aperture :

  1. using get_momentum_acceptance and passing the closed orbit and their reference points.
  2. using get_acceptance and passing the closed orbit and their reference points
  3. using get_acceptance and passing only the closed orbit.

All three finish without issues. The lifetime calculated from these three methods differs by less than 1%. The reason of this difference is not completely clear, but in any case, the modifications you made are OK.

I only have a doubt about a line that is removed where the variable 'istracked' is not longer assigned to True. It seems to have no impact but its name seems relevant.

@swhite2401
Copy link
Contributor Author

Yes I saw this small difference as well, my interpretation is that in one case you provide the full closed orbit looking with the default ring starting point and in the other case you rotate the ring and recompute the closed orbit for each repfts... it could be that the 6d orbit converges to slightly different values.

Concerning istracked pycharm complained about this line and in fact it is ok because this varaible is overwritten in the while loop and not used anywhere else so no need to initialize outside the loop.

@oscarxblanco
Copy link
Contributor

It's OK on my side. Thank you for this modifications !

pyat/at/acceptance/boundary.py Show resolved Hide resolved
@oscarxblanco oscarxblanco merged commit fc3c391 into master Aug 3, 2023
31 checks passed
@oscarxblanco oscarxblanco deleted the momap_orbit branch August 3, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants