search

ThorstenHellert / SC

6 stars 4 forks source link

PolynomBOffset #30

Closed oscarxblanco closed 1 year ago

oscarxblanco commented 1 year ago

Dear Thorsten,

is there are reason why PolynomBOffset and PolynomAOffset are not implemented ? SCregisterMagnets has it as an option but it is never used https://github.com/ThorstenHellert/SC/blob/e49b953bc3b216cff776247a0d89363fb0e76619/SCregisterMagnets.m#L32C1-L32C1

SCsetMultipoles adds the polynoms offsets, so, I guess it is supported on other functions but not registered in the SC structure.

ThorstenHellert commented 1 year ago

Dear Oscar,

the fields Polynom[A/B]Offset are typically used to store random multipole errors (see for example https://github.com/ThorstenHellert/SC/blob/master/SCsetMultipoles.m and https://github.com/ThorstenHellert/SC/blob/master/applications/ALSU_SR/setRndPolynoms_ALSU_SR.m). I see where your confusion comes from as it might be misleading in the description of SCregisterMagnets that Polynom[A/B]Offset are mentioned but here it only refers to all the SC related fields which the registered lattice element might have. I will think about how to make that more clear.

I see two issues with your suggestion of including it as explicit options in SCregisterMagnets (https://github.com/ThorstenHellert/SC/pull/29/commits/cfb48438dbb61227be9b65f6b24d41e0f8881cce): 1) I don't see a clear use case as it would add the same field offset to all magnets in that family and if you have different fields, why not making it the design value of that magnet family when defining the AT lattice element? 2) Right now you can call SCregisterMagnets with Polynom[A/B]Offset as optional parameters and since these fields are not explicit options of SCregisterMagnets (like 'HCM' or 'SkewQuad'), its values are interpreted as uncertainties from which random Polynom[A/B]Offset errors calculated and applied to each magnet when calling SCapplyErrors. Which effectively allows you to register random multipole errors directly in the registering process.

If we would go with your suggestion you can only add a specific field offset to all magnets within the registered family. I would say the current use case is more practical then the if we would go with your suggestion but maybe I'm missing something here?

oscarxblanco commented 1 year ago

Dear Thorsten,

thank you for the explanations. After some days of usage I think your point of view is right. I will remove the last commit.

Thank you again for your prompt response,

o