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

Cbs/curvealongz #461

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Cbs/curvealongz #461

wants to merge 6 commits into from

Conversation

smiet
Copy link
Contributor

@smiet smiet commented Nov 25, 2024

I wanted to add a toroidal field to a configuration, because I just wanted to jiggle the rotational transform a bit without too much ado. That would be nice with a straight coil along the z-axis, so I coded it up using the JaxCurve.

(Yes upon writing this PR I see that ToroidalBfield exists, but it does not allow one to shift the location of the curve along Z (playing with a 1/1 component), or optimize with it).

Think this might be useful for others, @krystophny has mentioned wanting to do something similar but resorting to using very large radius coils.

Some of the unittests are failing, mostly because the curve is not curved, and the evaluation of Kappa and it's derivatives fail. In that regard, what would be the best fix?

  • overload the kappa, torsion etc functions and their derivatives in the CurveAlongZ class with a default choice.
  • adapt the base Curve methods of evaluation of kappa, torsion etc and their derivatives to deal with straight sections.

option 1 would work, but lead to code repitition if other curves with straight sections are implemented
option 2 could lead to unneccesary computational overhead if every field evaluation nans need to be replaced

@andrewgiuliani @florianwechsung, any suggestions as to the best approach?

@smiet smiet requested a review from andrewgiuliani November 29, 2024 08:48
@krystophny
Copy link
Contributor

@smiet I opt for option 1. There will be no code duplication if you extract the required functionality into lower level functions that are used by several classes. Performance and simplicity of class hierarchy are higher priority than implementation details in special cases. Can you add that?

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