Adds Velocity, ShearStress, StressDiagonal, observables

[harveydevereux]

This adds a Velocity observable capable of averaging over supplied components x, y, z and atom lists. The behaviour is abstracted so also for stress we can obtain shear stress correlations $\langle \sigma_{xy}\sigma_{xy}\rangle+\langle \sigma_{yz}\sigma_{yz}\rangle+\langle \sigma_{zx}\sigma_{zx}\rangle$ with the built in ShearStress === Stress(['xy', 'yz', 'zx'])

• ci tests now test against post-process vaf, dependent on changes to it post_process uses full correlation, fix filter_atoms #284 .

• I used the ShearStress observable to get experimental viscosity for LiF which could be an example perhaps along with the VAF.

• still need to fix the docs/ update the developer guide.

• perhaps we could have a helper function to create Velocity-Velocity correlations from a structure (say, creating correlation_kwargs for different atom types automatically). As it stands Observable does not attach to a particular atoms object and so does not have this information at construction. Except implicitly via atoms: list[int].

• Velocity can take a SliceLike or list[int]. So open-ended ranges can be specified i.e. slice(0, None), but also non-monotonic, decreasing or whatever sequences can be specified that cannot be represented as a slice.

[ElliottKasoar]

Looks like a good start, thanks @harveydevereux!

I suspect I knew a lot of the answers to my questions when we discussed this when you started, but maybe it's helpful coming to it with fresh eyes, so we can make things as clear as possible.

[oerc0122]

Is this just list(self._components.values())?

[harveydevereux]

this may lead to duplicate indices. e.g. stress supports both "xy" and "yx" as self._components. self.components are the particular (say stress) components an observable "tracks" hence the need to access those particular values from the dict.

I've changed this so there is no allowed_components property, just a _allowed_components member to track this. Hopefully makes this clearer.

[oerc0122]

Don't like that this isn't StressShear to go with others. May also want StressHydrostatic rather than Diagonal?

[harveydevereux]

those are both good ideas,StressHydrostatic sounds good, unless someone can think of better I'll go with it

[oerc0122]

Blindly applying this isn't right.

You no longer need to check the if, as that's done by the operation. component_names is now just undefined and need to refer to the components which are not defined.

if any(self.allowed_components.keys() - components.keys()):
    raise ValueError(
                    f"'{','.join(self.allowed_components.keys() - components.keys())}' invalid, must be in '{', '.join(self.allowed_components)}'"
    )

[harveydevereux]

yes, unfortunately I did check the changes locally but then added them all together anyway... the code was odd due to the line length but this can be solved

[oerc0122]

It's my problem really, I click "suggestion" because I'm too lazy to match the formatting and then it renders with "add this" even when it's invalid code. I should change the type to Python

[oerc0122]

Suggested change

	" invalid, must be '{', '.join(self._components)}'"
	f" invalid, must be '{', '.join(self._components)}'"

[oerc0122]

I think this is a bit messy and confusing, personally would prefer something more like:

        stresses = sliced_atoms.get_stress(
                include_ideal_gas=self.include_ideal_gas, voigt=True
            ) / units.GPa
        return stresses[self._indices]

[harveydevereux]

I've split the index onto the return line as above, it is clearer.

[oerc0122]

It might be best to move the list case into slicelike_len_for so we can have a clean interface here. Maybe selector_len or something in that case?

[harveydevereux]

done, but this also made me realise slicelike_len_for is not defined due to the import guarding (ultimately a circular import issue from before). @ElliottKasoar I have moved CorrelationKwargs into correlator.py so there is no longer an issue with Observable and SliceLike circular imports.

[ElliottKasoar]

Is it worth checking if index is a StartStopStep (or any other valid input)?

[harveydevereux]

do you mean throwing an error if it is not actually a SliceLike?

i.e. these cases are no good but "run"

>>> slicelike_to_startstopstep([1,2])
[1, 2]
>>> slicelike_to_startstopstep((None, None, None))
(None, None, None)

its probably a good move

[harveydevereux]

We could do

def validate_slicelike(maybe_slicelike: SliceLike):
    # ...
    if isinstance(maybe_slicelike, (slice, range, int)):
        return
    if isinstance(maybe_slicelike, tuple) and len(maybe_slicelike) == 3:
        start, stop, step = maybe_slicelike
        if (
            isinstance(start, Optional[int])
            and isinstance(stop, Optional[int])
            and isinstance(step, int)
        ):
            return

    raise ValueError(f"{maybe_slicelike} is not a valid SliceLike")

?

[ElliottKasoar]

Yes, that sort of thing would be great

[ElliottKasoar]

Not really work for this PR as such, but how easy would it be to integrate/use these in post processing, rather than on-the-fly calculations?

[harveydevereux]

Not really work for this PR as such, but how easy would it be to integrate/use these in post processing, rather than on-the-fly calculations?

All it needs is to be called on some sequence of atoms objects. But if you are post-processing why not just use post-process? The only advantage of these is they extract the data from Atoms in this case

[ElliottKasoar]

All it needs is to be called on some sequence of atoms objects. But if you are post-processing why not just use post-process? The only advantage of these is they extract the data from Atoms in this case

Most significantly, I'm thinking about how easy it is to extend postprocess to observables we've defined, but aren't supported yet.

At the moment we can only do RDFs and VAFs, so if we didn't calculate any other observable on the fly, we might still want to be able to use an Observable.

It could also fit into unifying the interfaces e.g. for the VAF, depending on how their efficiencies compare, which would avoid questions about their consistency any time we make a change.

[oerc0122]

Just so we know it's being run for side-effects.