prisms-center / CASMcode_configuration

CASM configuration comparison and enumeration
Other
0 stars 5 forks source link

Why separate Configuration and ConfigurationWithProperties objects? #22

Closed jli5373 closed 2 months ago

jli5373 commented 3 months ago

I was wondering why we have these two separate classes since it seems that ConfigurationWithProperties is just Configuration with 2 extra properties options. Is there a reason why we can't just have a default Configuration be what ConfigurationWithProperties is now? (maybe with the option for those options to be empty)

bpuchala commented 3 months ago

This is a very reasonable question, and in fact the CASM v1 Configuration includes both DoF and properties (and even allows storing multiple sets of properties calculated by different approaches). However, I came to find it to be too much extra work - both for users and developers - to understand and explain what happens in a method that takes a Configuration, without enough of a benefit.

All of the methods that act on a Configuration, particularly comparison and transformation by symmetry operations, but then also things that use those operations like enumeration, symmetry analysis, make primitive, make super configuration, make canonical, etc. become more complicated if the Configuration class includes properties - even though usually properties are not included - because either they have to deal with the properties, or they have to document / check that they don't work on properties. This can be handled - but in fact a lot of the time in v1 it was just ignored - and then when you came across a case where properties might make a difference you had to go in and look at the code to find out if the properties would be affected in a way you wanted or didn't want.

In practice, configurations with properties are only used in a fraction of the cases where we do something with configurations - primarily, we sometimes want to be able transform the properties by symmetry operations, for instance to match the orientation and translation of an existing configuration after import and mapping; or to convert to/from a structure and assign properties like displacements to a particular site.

When we want to bring both configuration and properties to fitting / analysis we often also want to bring other things (name, correlations, mapping scores, calculation source, etc.) anyways, so in my view the benefit is not so much fewer data structures - because we have to deal with additional data structures anyways - but there is a benefit when it makes unambiguous what site a local property is associated with, and what orientation the properties have relative to the DoF.

So in transitioning methods to v2 I removed the properties from Configuration and added the new ConfigurationWithProperties class. With this approach, usually we just have a Configuration and then it is clear nothing is done with properties because there are no properties. If something is going to be done with properties, then the method takes a ConfigurationWithProperties and it is clear that the properties will be taken into account.