GetStrain function

[f-iniv]

Function to obtain strain matrices from a desired stress matrix using the elastic tensors from a crystal

Added a function to convert from the elastic to the Compliance tensor, and definitions for monoclinc and trigonal elastic tensors

[dkriegner]

Thanks for the contribution. this looks interesting.

I would propose few changes before I am willing to merge it tough. In particular I think the elastic parameter of GetStrain should be either dropped or made optional. The Crystal object already should possess all the required elastic parameters in the cij property. Could you check this?

Also I think it would be good to conceive some unit tests to continuously test that this function gives sensible results. Would you be able to provide a unit test for this new function?

[f-iniv]

I made the cij property as the standard, removed elastic, changed sig as a matricial input to match ApplyStrain and also added unit tests for the stress and elastic tensors.

If you think anything else needs change, just let me know.

[dkriegner]

thank you for the additional work.

I have few more comments: 1) I think the docstrings of GetStrain and ApplyStrain should mention that the other method exists so that users can find the link between those to related functions easier 2) If I am not mistaken, the implementation of GetStrain could be much simpler (and faster) using einsum. I think the loops can be avoided by this function. Can you look into this? 3) Regarding the unittests (although your changes are good) I was actually thinking more of test cases like those implemented in https://github.com/dkriegner/xrayutilities/tree/main/tests. Can you think of something one could include there to ensure this functions (keep) work(ing) correctly? I like to include some tests because they can easily reveal mistakes in future code changes.

[f-iniv]

Indeed using einsum was way simpler, it all ended up becoming a single line. I also reduced the size of 'Cij2Sijkl' by using 'Cij2Cijkl' as part of it.

For the unittests, the only way I thought of implementing them was a sort of recursive way, by creating a 'GetStress' function, testing one against the other and having a test for both.

[dkriegner]

Thanks for the improvements. The implementation is now cleaner. I unfortunately still have more comments, but they are getting smaller:

The GetStrain and GetStress functions should likely be methods of Material. Since in Material also all cij matrix is already defined. (Also in Python the self argument is common only for methods of a class). Or do you see any reason this needs to be a normal function?

Do you think the docstrings should mention the units which should be used for the stress? Of course it depends on the units of cij which now we try to use consistently as N/m2.

Also could you add your name to the copyright statement at the beginning of the files you changed? (I will add a note on that to the contributing file in the main folder)

[f-iniv]

No worries!

The way I was using it previously was a function, but there's no reason for it not to be a method, so I already fixed it.

I also agree on mentioning the units used for the stress, initially I intended to add that, but eventually forgot.

[dkriegner]

thanks again for the work on this. If you have other ideas what to change feel free to submit more PRs! your addition made me also realize that there are some shortcomings in the Poisson ratio (and other mechanical parameters) implemented in Material. I think these are not used anywhere else in the code, but I guess one should get them correctly from the full anisotropic elasticity tensor. (see e.g. issue #139)