Minor simplification of trim_cell

[atztogo]

The nearest integer matrix of axis_inv is tmat_p_i at line ~520 in function translate_atoms_in_trimmed_lattice. I prefer the later because I want to have a transformation matrix to be an integer matrix or inverse of an integer matrix whenever possible.

Actually, axis_inv is unnecessary to be computed in translate_atoms_in_trimmed_lattice because it is same as tmp_mat in function trim_cell that calls translate_atoms_in_trimmed_lattice and translate_atoms_in_trimmed_lattice is called only from trim_cell.

Although axis_inv and tmat_p_i can be slightly different if the input cell is distorted, I would like to suggest this change.

[codecov-commenter]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (a96d72a) 83.56% compared to head (c7de271) 83.55%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #347      +/-   ##
===========================================
- Coverage    83.56%   83.55%   -0.01%     
===========================================
  Files           24       24              
  Lines         8157     8154       -3     
===========================================
- Hits          6816     6813       -3     
  Misses        1341     1341              

Flag	Coverage Δ
c_api	76.43% <100.00%> (-0.01%)	⬇️
fortran_api	56.29% <100.00%> (-0.02%)	⬇️
python_api	80.42% <100.00%> (-0.01%)	⬇️
unit_tests	1.24% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/cell.c	74.56% <100.00%> (-0.27%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lan496]

I also agree to use integer matrices as possible. BTW, can you write a description for Cell *cel_trim_cell(...)? After reading, I understand that this function computes a mapping of atom positions in a small cell trimmed_lattice. I feel this is a bit far from its function name.

[atztogo]

@lan496, thanks for your review. I added some descriptions.

BTW, can you write a description for Cell *cel_trim_cell(...)? After reading, I understand that this function computes a mapping of atom positions in a small cell trimmed_lattice. I feel this is a bit far from its function name.

Do you mean the word trim is not intuitive?

[atztogo]

My line length setting is considered different from the original code. Do we have a formatter to make it same?

[LecrisUT]

Are you using clang-format by default? I'm not sure on VSCode, but on CLion there is an option to use clang-format instead of the built-in

[atztogo]

Thanks for your info. @LecrisUT. I use VSCode, and my current setting is like below,

    "C_Cpp.clang_format_style": "{BasedOnStyle: Google, IndentWidth: 4}",

I found some information, https://code.visualstudio.com/docs/cpp/cpp-ide#_code-formatting . I should read this in detail.

[lan496]

Do you mean the word trim is not intuitive?

Yes, I felt nonintuitive because Cell *cel_trim_cell already gets trimmed_lattice (so called "trimmed lattice") as an input.

Cell *cel_trim_cell(int *mapping_table, const double trimmed_lattice[3][3],
                    const Cell *cell, const double symprec)

[lan496]

Sorry, I missed the returned trim_cell for this function. So, the function name should be fine.

[atztogo]

Clearly the code is confusing and we should refactor it, but not in this PR.