fix(layergroup): fix unique axis of monocli system

[site-g]

layer_laue2m stored the two-fold rotation axis in axis[0], while laue2m store it in axis[1]. del_layer_delaunay_reduce_2D was Delaunay reducing a rectangle surface of the cell for layer group cases before.

[codecov-commenter]

Codecov Report

Base: 85.34% // Head: 85.34% // No change to project coverage 👍

Coverage data is based on head (763447c) compared to base (2b06c29).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #209   +/-   ##
========================================
  Coverage    85.34%   85.34%           
========================================
  Files           18       18           
  Lines         1358     1358           
========================================
  Hits          1159     1159           
  Misses         199      199           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[atztogo]

Could you write more detailed comment?

[site-g]

2D Delaunay reduction is conducted to transform the oblique face of monoclinic cell, i.e. the face vertical to the two-fold rotation axis, into a Delaunay parallelogram. As in layer group, the unique axis for conventional cell is a or c rather than b, I set a to be unique axis in layer_laue2m and move it to c for monoclinic/oblique cells after del_layer_delaunay_reduce_2D

spglib/src/spacegroup.c

Lines 986 to 990 in 2b06c29

	/* For layer with 2D mp lattice, move the unique axis to c */
	if (aperiodic_axis_conv == 0) {
	mat_multiply_matrix_d3(smallest_lattice, smallest_lattice,
	change_of_basis_monocli[2]);
	}

So for space group parameter unique_axis of del_layer_delaunay_reduce_2D is 1, while for layer group it should be 0.

[atztogo]

Thanks @site-g.

a, b, c axes correspond to 0, 1, 2 in the code.

I think this PR can be merged. How do you think @lan496?

[lan496]

@atztogo I do not fully understand the implementation of 2D Delaunay reduction, so I can only say it seems to make sense.
At least, there were not any test cases to detect this issue before, were there?

[atztogo]

2D Delaunay reduction is simply a triangulation. If this PR will not break space group implementation, it is approximately OK for me.

We have almost no tests, which is not only layer group. The tests for layer group must be written, too, but since I don't understand its implementation well, it is difficult to tell how to write the test to @site-g right now. In this specific case, I think @site-g has a table how the axes are chosen, so the test should be able be written following the table. @site-g, information about the test in spglib is written here, https://github.com/spglib/spglib/tree/develop/test, which is written by @lan496. Writing tests and docs on code requires some experience. Unless the previous experience on it, it would be uneasy.

My plan to make this implementation of the layer group (and also whole spglb) maintainable is

1. Summarize our methodology and implementation in a documentation, which is lead by @site-g. I know @site-g already has some.
2. Refactor the code of the layer group and write tests for space group and layer group little by little. I have many variables and functions that I don't understand how they work in the layer group implementation. Some information close to the code should be written in the code, for which we will standardize how to write it. This will take time. I will need to have enough time that I can concentrate on kicking off this step.

[site-g]

@lan496 I am preparing an appendix about Delaunay reduction, comparing the difference in space and layer group case. It may be added to Overleaf tomorrow.

@atztogo I am tabulating the axes choice for layer group, too. However, I face some difficulty with the orthorhombic lattice. As shown in seekpath #57, the freedom of the axes is determined by the index of L(g) in N_A(g). I can understand it, but cannot give a rather rigorous derivation. Can you give me some suggestions?

I am not familiar with GoogleTest and need time to learn it. That might not be very fast.

[lan496]

Thanks to site-g's document for Delaunay reduction, I now have read the point group search (ptg_get_transformation_matrix) and Delaunay reduction (del_layer_delaunay_reduce_2D) for layer group. I think this PR is ready to be merged. After this PR is closed, I'll create a PR to add some comments and small unit tests on layer-group implementation, which are by-products of my code reading.

@atztogo As for axes choices, I cannot find a description of spacegroup.c:num_axis_choices_ortho or spacegroup.c:change_of_basis_monocli in the arxiv's paper. It is planned to describe them in a submitted version?

[atztogo]

@atztogo As for axes choices, I cannot find a description of spacegroup.c:num_axis_choices_ortho or spacegroup.c:change_of_basis_monocli in the arxiv's paper. It is planned to describe them in a submitted version?

It's a good point. I haven't planned it. If I remember correctly, this is a little bit tricky. I will look at it, and hope to describe it in another issue.

[atztogo]

If I remember correctly, spacegroup.c:num_axis_choices_ortho or spacegroup.c:change_of_basis_monocli gives the number of different generators that change basis vectors found in Euclidean normalizer section of ITA 2016 Table 3.5.2.4. The numbers were collected by my eye checking "Further generators".

These information is used to find transformation matrix corresponding to Hall symbol with some constrains. So this is in the step (i) of spglib arxiv manuscript, but as @lan496 pointed out, it is not written. "Some constraints" means controlling order of a,b,c. If Euclidean normalizer allows change of (a,b,c) basis is applied to let, for example, |a|<|b|, |a|<|c|, |b|<|c| as much as possible.

In this way, a (transformation matrix P, origin shift p) is determined. But different (P, p) can be obtained by space group operations, therefore, spglib chooses one of equivalent (P, p) under some preference about orientation to make conventinal basis vectors closer to spglib-standardized basis vectors in Cartesian coordinates, and origin shift distance shorter, which is done in the following function,

spglib/src/refinement.c

Line 1417 in 2b06c29

int ref_find_similar_bravais_lattice(Spacegroup *spacegroup,

[lan496]

Merged. Thanks, @site-g !
I've opened a new issue #220 to continue to discuss on axis choices.