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

Feature/misalign coils #13

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Conversation

renereimann
Copy link
Collaborator

Implement rotation in (r,z) and shift in r for misalignment studies.

…tialized to 0 so everything should be backward compatible. r0 shifts the coil in r, so we can simulated uncertered coils. The implementation is straight forward since we evaluate B(r-r0). In addition alpha does a tilt, by rotating from a primed lab/simulation frame to the unprimed frame that is aligned wiht the coil. The rotation has to work on the (z',r')->(z,r) vector and the resulting field has to be rotated in opposite direction (Bz, Brho) -> (Bz', Brho'). These modifications allow for studing misalignment of coils.
@@ -62,6 +67,13 @@ def evaluate_B(self, pos, derivatives=False):

Bz[...] = C/(2*alpha**2*beta)*((a**2 - r**2)*E_k2 + alpha**2*K_k2)

Copy link
Owner

Choose a reason for hiding this comment

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

Could gate the new raw arrays by if self.alpha!=0: Saves some time and memory. Unfortunately, CRESana has a big memory footprint already if it is run with a long trajectory. If I recall correctly it does not directly matter with the current setups that I use because the B_field evaluation is done with the Interpolation function. Without the interpolation function the line B_raw=np.empty_like(pos) will create a new array of size n_samples*n_antennas during the simulation.

Copy link
Owner

@MCFlowMace MCFlowMace left a comment

Choose a reason for hiding this comment

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

It looks straightforward to me. But I think the field would not be rotationally symmetric anymore, right? Some parts of the trajectory simulation needs the assumption of rotational symmetry. I suggest we add a warning with the python warnings package to the coil class if the new parameters are used that tells that it might be incompatible for the simulation. Here is an example where I use the warnings package for a deprecation warning https://github.com/MCFlowMace/CRESana/blob/feature/misalign_coils/cresana/trap.py#L321
Should be a different warning type.

Also it would be nice if the visualization of the setup, from here
https://github.com/MCFlowMace/CRESana/blob/feature/misalign_coils/cresana/bfield.py#L455
would be compatible with this change. I suggest to move the calculation of the line of a coil to the Coil class itself and add a function that can be used in visualize to ask for the line.

@renereimann
Copy link
Collaborator Author

I implemented all three suggestions, ready for re-review

@MCFlowMace
Copy link
Owner

I just looked at the visualization of a coil with r offset and a rotation angle using the visualize function. I'm not sure if the result is expected. I expected the coil moves to a different r position and is rotated in place. But this is not what happens. Using alpha=pi/2 for example the coil is rotated as I expect but it moves to z=r0. I think the order of translation and rotation is messed up. I want to ask you to check this again please.

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