Make tensors non-symmetric by default

[Krzmbrzl]

As long as the calling site doesn't explicitly specify a symmetry or hermiticity, the tensor will be entirely non-symmetric. This conforms to what a typical user will expect.

In case we do have a symmetry, we don't silently overwrite it, even if we know it doesn't make sense. Instead, throw a proper error informing the user that they can't do this. This way, they will always know what is happening.

[Krzmbrzl]

Turns out quite a few of our unit tests depend on the default tensor symmetries (which is probably not that great). This needs to be fixed but I am done for today. If anyone wants to feed this task to an AI agent, just drop a comment and I will be happy not to do it manually. I guess the correct way of fixing most of this is explicitly setting the default symmetries for deserialization.

[evaleev]

@Krzmbrzl I am converging towards having Context control the defaults. The default context should indeed default to non-symmetric throughout, but user knows best, so they should be able to override the defaults for ergonomics. Thoughts?

[Krzmbrzl]

@evaleev I'm not too convinced of that. This implies that the behavior of existing code can be changed by changing the used context. In particular, if some internal implementation instantiates a tensor without specifying symmetry simply because the tensor doesn't have symmetry then this code would be broken if a user altered the default symmetry via context.

One could of course argue that all code should just always declare the symmetries as needed but then I think we shouldn't have a Tensor constructor that doesn't require full symmetry specification. However, this seems more cumbersome to me than having to specify symmetry explicitly if symmetry is present.

In short: As long as we allow construction of Tensors without full symmetry specification, I think it's not a good idea to change the default symmetries via context. And if we always require full specification there are no default symmetries being used in the first place.

[evaleev]

@evaleev I'm not too convinced of that. This implies that the behavior of existing code can be changed by changing the used context. In particular, if some internal implementation instantiates a tensor without specifying symmetry simply because the tensor doesn't have symmetry then this code would be broken if a user altered the default symmetry via context.

True. But, while preventing blind copy-pasting, the impact on users should be limited. Most beginners will not know about context other than configuring spaces. The expert users will appreciate the ability to tune the defaults (thus for those who write code by hand or want to fit snippets on their slides). And the advice in both cases (whether defaults are compile-time or runtime-time controller) is the same: if you want "forever-correct" code you must hardcode all parameters explicitly.

One could of course argue that all code should just always declare the symmetries as needed but then I think we shouldn't have a Tensor constructor that doesn't require full symmetry specification.

Verbosity-by-default is good, but again teaching etc. calls for reasonable defaults; newbies get distracted with non-essential details.

[Krzmbrzl]

But, while preventing blind copy-pasting, the impact on users should be limited.

The issue I am concerned with is mostly the impact on SeQuant developers. Making mistakes with symmetry is just so much easier if you might end up having different defaults than someone using SeQuant.

The expert users will appreciate the ability to tune the defaults (thus for those who write code by hand or want to fit snippets on their slides).

I'd argue that expert users will simply create a wrapper function if they want to create tensors with specific default symmetries.

And the advice in both cases (whether defaults are compile-time or runtime-time controller) is the same: if you want "forever-correct" code you must hardcode all parameters explicitly.

Well, that depends on the philosophy behind defaults, I guess. As an extreme case, if I default construct a std::string I cam expect it to be empty even if I didn't explicitly construct it from an empty string.

Verbosity-by-default is good, but again teaching etc. calls for reasonable defaults; newbies get distracted with non-essential details.

Yeah, my intention wasn't to suggest that we switch to always requiring the full symmetry spec. But I do believe the default should be nonsymmetric under all circumstances.

---

Bottom line is that I'd vote that if we do have a facility that adapts defaults based on context, we make it an explicit opt-in. Something like a tensor factory that we could name SymmTensor or something like this. But I would really like to avoid having Tensor change defaults under my feet 👀

[evaleev]

The issue I am concerned with is mostly the impact on SeQuant developers. Making mistakes with symmetry is just so much easier if you might end up having different defaults than someone using SeQuant.

By default we will have the default to use nonsymmetric tensors, as you intended. The use of runtime-adjustable defaults is to support legacy code (in tests and e.g. in MPQC). Sorry, we already have too much code in MPQC that is likely to break if we change the defaults. Adjusting it is not practical.

Yeah, my intention wasn't to suggest that we switch to always requiring the full symmetry spec. But I do believe the default should be nonsymmetric under all circumstances.

I agree. Nonsymm by default. I just argue we need runtime control for the defaults.

Bottom line is that I'd vote that if we do have a facility that adapts defaults based on context, we make it an explicit opt-in. Something like a tensor factory that we could name SymmTensor or something like this. But I would really like to avoid having Tensor change defaults under my feet 👀

Sorry, there is too much "legacy" code already.

[Krzmbrzl]

Doesn't that legacy code expect Conjugate braket symmetry and nonsymmetric for everything else? And to my understanding Conjugate never affected the processing of equations at least on SeQuant side 👀

And if the purpose is only to support legacy code, could we make that semantically more obvious? E.g. don't have publicly accessible fields in the context that suggest that this is a normal, supported feature but instead have a dedicated, scoped LegacyMode object that sets these context fields via internal accessors. Using a construct similar to this would make it more obvious that unless you need it for legacy reasons, you shouldn't use it. 🤔

[evaleev]

Doesn't that legacy code expect Conjugate braket symmetry and nonsymmetric for everything else? And to my understanding Conjugate never affected the processing of equations at least on SeQuant side 👀

the column symmetry was always assumed.

And if the purpose is only to support legacy code, could we make that semantically more obvious? E.g. don't have publicly accessible fields in the context that suggest that this is a normal, supported feature but instead have a dedicated, scoped LegacyMode object that sets these context fields via internal accessors. Using a construct similar to this would make it more obvious that unless you need it for legacy reasons, you shouldn't use it. 🤔

I think there are valid reasons to provide the runtime control over defaults, e.g. teaching/demos. So LegacyMode becomes smth less forbidding, thereby defeating the purpose. We can just say "don't use unless you are expert" in the docs. We could deprecate these methods so that the use generates noise, but most ppl will ignore the noise.

[Krzmbrzl]

Does that actually have to change the default symmetries of tensors inside SeQuant implementations? Or is it intended to change defaults of explicitly user-constructed tensors only? I see how the latter could be useful but I currently don't see a use for the former where changing the symmetries could easily break the code 👀

not sure I quite get the difference. Bottom line it's a lot easier to read t{a1,a2;i1,i2} than t{a1,a2;i1,i2}:N-C-S, so ideally for a given domain defaults can be defined once so that the noise is reduced. The defaults should be consistent, so the programmatic defaults and deserialize defaults should match, and be runtime-controllable.

If we make the defaults changeable, we need to ensure that all test cases are run with different defaults (ideally all possible combinations though that might become excessive) to ensure that we never use these defaults internally (aka. to ensure we are always explicit about all tensor symmetries).

New/nonlegacy tests should follow the best practices and hardcode the symmetries explicitly. Only tests that use e.g. publication snippets (where symmetries for the sake of brevity may be implicit) or specifically testing the defaults will omit symmetries. But in such case the block of tests that imply specific defaults will be enclosed in a scoped context manipulator.

[evaleev]

@Krzmbrzl are we converging on consensus? Or do we need to discuss during next SeQuant telecon?

[Krzmbrzl]

I think we should discuss the details during the next meeting.

An important distinction for me is whether we are talking about the defaults for parsing or the defaults for constructing a tensor directly. I'm much more open to have parsing defaults be runtime-adjustable than tensor constructor defaults. The reason being that parsing basically never happens in SeQuant's internals (in the implementation of typical processing routines) but is almost exclusively done outside in "userspace". Hence, the chance of changes to these defaults messing with the correctness of functionality implemented in SeQuant are quite slim. As far as tensors constructed directly in the code are concerned, this definitely happens in the internals and thus can easily screw up correct behavior (which can be devastating if the user doesn't have a direct way to verify that the resulting expressions are what they expect them to be).

[evaleev]

superceded by #557