Skip to content

Conversation

electronsandstuff
Copy link
Contributor

This PR adds an accompanying regression test and minimum working example for #1762.

@DavidSagan
Copy link
Member

@electronsandstuff Looks like there are conflicts there that need to be resolved. Note: I did just add a test for this on my side.

@electronsandstuff
Copy link
Contributor Author

OK, resolved if you want to add this one too.

@DavidSagan
Copy link
Member

@electronsandstuff Looks like now there are failing tests...

@electronsandstuff
Copy link
Contributor Author

I am checking on my end and there still seems to be an issue in the cavity. That's why the new unit test is failing. Here is Bmad 20251016.0 on the test lattice.
image
Here is Bmad 20250915.0
image
I think something is still going on with the energy calculations, but I also don't have permissions to re-open the issue.

@DavidSagan
Copy link
Member

I assume by "issue" you mean #1762. See my comment at the end of this issue. The fact that there is a difference now as compared to 20250915.0 is to be expected.

@electronsandstuff
Copy link
Contributor Author

Sorry if I am misunderstanding something, but I am still confused.

I know that E_tot is the reference energy, but since the lattice doesn't have any pz orbit in either Bmad version, it is also the energy of the bunch.

In the older Bmad version, the code says that the bunch gains 100keV through the cavity. In the latest Bmad, the bunch's energy doesn't change (no change in E_tot or orbit pz). Outside of convergence issues, I would expect there to be a consistent value for the beam's energy after the cavity. Maybe I am calculating it incorrectly or missing something?

@DavidSagan
Copy link
Member

Yes there has been a change and the results are not consistent. What the code did in the past was not what a User would expect so this has changed.

@DavidSagan
Copy link
Member

If you want we can zoom and I can explain in detail.

@electronsandstuff
Copy link
Contributor Author

OK, thanks. I will reach out to you over slack. In the meantime it sounds like this test isn't correct and I can close out the PR.

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