fix_all()

[andrewgiuliani]

fix_all() only fixes the dofs associated to the current Optimizable, and not it's ancestors.

This is confusing when working with, e.g., ScaledCurrent. Calling fix_all() on a ScaledCurrent does not fix the current at all.

After discussing with @florianwechsung, a proposed change would be to introduce a fix_full() or fix_local() to distinguish between these two use cases.

[landreman]

For other functions and attributes of an Optimizable object, full refers to "including both fixed and free dofs" rather than "including ancestors". See e.g. https://simsopt.readthedocs.io/en/latest/optimizable.html#function-reference. The proposed fix_full would break this pattern so maybe we should find a different name. Looking at the table of other functions, the pattern is to have local_ in front of functions that exclude ancestors and to have no prefix in front of functions that include ancestors. That means the names that would best preserve the pattern would be fix_all() when including ancestors and local_fix_all() when excluding ancestors.

@andrewgiuliani @florianwechsung @mbkumar what do you think?

Sorry I didn't catch this sooner.

[mbkumar]

Yes, fix_full is breaking the naming conventions employed. @landreman, I like your proposal. An alternative would be fix_global and fix_local.

[andrewgiuliani]

I like Matt's proposal

[mbkumar]

Fixed in #222

[mbkumar]

Agree it is confusing. Will issue a fix.

On Fri, Apr 15, 2022 at 3:38 PM Andrew Giuliani ***@***.***> wrote: Assigned #215 <#215> to @mbkumar <https://github.com/mbkumar>. — Reply to this email directly, view it on GitHub <#215 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA62VEH76JCLF42VJ76DVFLVFHAU3ANCNFSM5TREJACQ> . You are receiving this because you were assigned.Message ID: ***@***.***>

-- Bharat Medasani Engineer Princeton Plasma Physics Lab (PPPL)