More flexible solenoid definition, resolve issue 1147

[HelmutCERN]

Allow for more flexible solenoid definition based on the two independent parameters, strength and length
Strength can be ks or ksi (ks*l)
and length l for a thick or lrad for the thin solenoid.
Backward compatible : as before, the thin solenoid can also be defined with L=0, and providing both ks and ksi. In this case Lrad is calculated from ks and ksi.

Code changes:
mad_elem.c mad_elem.h
new check_set_consistent_solenoid
for more tolerant solenoid definition
thick solenoid with L>0
and thin with Lrad>0
on the make_element level
ks is calculated when not specified or inconsistent from ksi/lenth

mad_node.c
Save attributes when modified by check_set_consistent_solenoid

Further changes also of more general interest :
mad_mkthin.cpp
Zero length elements are not sliced.
They can still get a slice number > 1 by selection which can confusing when looking at the sliced sequence.
Using a new code piece
zero_length_elements_1_slice(el_list* the_element_list)
called in makethin after set_selected_elements
the slice number is now kept at 1 for zero length elements,
and thick and slice number removed as active attribute in copy_thin.

mad_dict.c
missing slice number added to rfcavity

[rdemaria]

Since there are not many solenoid users, how do you see if we allow only KSL (un purpose different from KS and KSI) in combination with L and LRAD. I see an advantage in breaking compatibility, but bringing in a simpler logic.

If you see too disruptive, let's keep this solution.

Since you were faster than me could you also add the small fix of the cavity in this PR?

[HelmutCERN]

Dear Riccardo, dear all Yes, I agree that a definition with a single new strength parameter KSL and one length L for thick or LRAD for thin would be simpler, clearer and avoid inconsistencies. The new KSL should then also go to the twiss table replacing the current KS (in fact dummy, always 0) and KSI. This would however break backwards compatibility (causing an error reading LHC sequences). Probably a good idea to discuss this first in a (LNO) meeting. Possible compromise : Go to the new definition, but allow for some time for the older definitions such that LHC sequences would not fail, just writing a warning that this should be changed to the new definition. The pull request includes in fact also the small cavity fix. Cheers, Helmut On 12 May, 2023, at 18:06, Riccardo De Maria ***@***.******@***.***>> wrote: Since there are not many solenoid users, how do you see if we allow only KSL (un purpose different from KS and KSI) in combination with L and LRAD. I see an advantage in breaking compatibility, but bringing in a simpler logic. If you see too disruptive, let's keep this solution. Since you were faster than me could you also add the small fix of the cavity in this PR? — Reply to this email directly, view it on GitHub<#1179 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEUCRLXHMK2KPXFFCTDWW5LXFZNZPANCNFSM6AAAAAAX7WJOGY>. You are receiving this because you were assigned.Message ID: ***@***.***>

[rdemaria]

Ok for your proposal. As an alternative, we could consider adding a ThinSolenoid element that takes KSL and LRAD only and keep the Solenoid with KS and L. Could this work for you? In twiss tables I would use KSL for both as we do with K1L for instance.

[HelmutCERN]

Dear Riccardo, dear all Your alternative proposal is fine with me. I like in particular the idea to define a new element THINSOLENOID twiss.f90 has in fact already a separate routine SUBROUTINE tmsol_th for the thin solenoid and the definition of the new thin element avoids inconsistent attributes. My only reservation is on renaming the integrated solenoid strength from ksi to ksl. mad_dict.c has in fact : "ksi = [r, 0], " /* was: ksl, but that clashes with naming conventions of multipoles */ ksi is also used in tracking and ptc. On the input level: label: SOLENOID, L=real, KS=real; label: THINSOLENOID, LRAD, KSI=real; Twiss table, single integrated strength KSI value for both. For backwards compatibility still allow to read (maybe with warning, no error) label: SOLENOID, L=0, KS=real, KSI=real; as THINSOLENOID I could see that I prepare a new fork for this in the next days for testing. Cheers, Helmut On 13 May, 2023, at 9:06, Riccardo De Maria ***@***.******@***.***>> wrote: Ok for your proposal. As an alternative, we could consider adding a ThinSolenoid element that takes KSL and LRAD only and keep the Solenoid with KS and L. Could this work for you? In twiss tables I would use KSL for both as we do with K1L for instance. — Reply to this email directly, view it on GitHub<#1179 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEUCRLVDOCHLJINZ2CORT7DXF4XF7ANCNFSM6AAAAAAX7WJOGY>. You are receiving this because you were assigned.Message ID: ***@***.***>

[rdemaria]

I see what you mean with KSL vs KSI now. Although I don't see a specific issue with KSL in solenoid, I see a potential issue if somebody check for KSL to determine if it is a multipole. So ok to keep KSI.

[ldeniau]

Here is what madng does when entering a quadrupole, same rule for elements thin and tick.

if l == 0 and abs(k1)+abs(k1s)+abs(k0)+abs(k0s) > 0 then
  warn("quadrupole '%s' defines strength with zero length", elm.name)
end
knl[1], ksl[1] = knl[1]+k0*l, ksl[1]+k0s*l
knl[2], ksl[2] = knl[2]+k1*l, ksl[2]+k1s*l

This very simple rule makes the overall code very simple. The elements maps are aware that they are thick or thin and use knl and ksl correctly and convert them to local strength if needed using l or lrad appropriately.

[HelmutCERN]

The pull request now includes the fix for the issue # 1180

[HelmutCERN]

The differences in test-thick-dipole-2 and test-thick-dipole-3 reflect the improved naming/cleaning and the generated thin.seq should become the new thin.seq.ref. To my knowledge no more remains issues - ready to merge.

[rdemaria]

Thanks a lot! Can you commit the new .ref? This will retrigger the tests then I can merge

[HelmutCERN]

New references pushed, slicing related tests now look all ok

[rdemaria]

Delaying tests on windows.