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

Time step option and adapting tests #6

Merged
merged 23 commits into from
Jan 30, 2025

Conversation

fsoubelet
Copy link
Collaborator

I have added the possibility of giving a specific time step for the iterations. I have also added a few more safeguards in the functionality's logic as we were for instance assuming that a constraint of "coupling" or "excitation" was provided in the loop when the user could just provide None, which would crash the program. This is now handled.

The tests have been adapted to the new APIs, and I made a fixture to provide the configured line (module scope as it is untouched by tests themselves).

Rather than the previous split I have chosen to have a test function for each constraint mode (coupling, excitation and none) and to parametrize over the emittance_coupling_factor or simply the starting emittances. For comparisons I have used the formula from the scripts I put in previous PRs, which is equivalent to that of the replaced tests, just a different form.

Tests require some changes that I made in a branch for Xtrack (namely, incorporating our functionality into the TwissTable) but that's about it.

I will do, for a later PR, tests of behaviour. We can also add hard-coded tests against ELEGANT results once that is available.

@fsoubelet fsoubelet added the enhancement New feature or request label Jan 29, 2025
@fsoubelet fsoubelet self-assigned this Jan 29, 2025
@SebastienJoly
Copy link
Owner

In the file "test_ibs_equilibrium.py" you made a typo at each occurrence of the word "iterations". You have written "ierations" instead. Otherwise for the ref for the analytical formulas. I have two references where they are described. The problem is that, in the one treating the case $j_x \neq j_y$, they have made a mistake in the way they define their emittances.
Either we use them or we can go full ego trip and reference our next IPAC paper. Apart from that great tests!

No comment regarding the changes in "equilibrium.py".

@fsoubelet
Copy link
Collaborator Author

I will accept shame for 15 generations.

For the reference this is an offhand into the docstring of a test function so I don't think it matters too much. We can simply ref and mention that there is an error in the emittance definition for instance, then add the IPAC paper later.

@fsoubelet fsoubelet merged commit 3cbb2fa into feature/equilibrium_emittance Jan 30, 2025
@fsoubelet fsoubelet deleted the tstep_and_tests branch January 30, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants