Enhance cell editor

[superstar54]

fixes #365

• editing cell vectors directly.
• cell transformation using a 3x4 transformation matrix.

[unkcpz]

@superstar54 Thanks! That "Cell vectors" and "Cell transformation" duplicate each other? Why not just keep the "Cell transformation"? I would suggest adding instructions on how the matrix apply to the cell.

[superstar54]

That "Cell vectors" and "Cell transformation" duplicate each other?

No. The "Cell vectors" are the cell itself (3x3 array). This enables the user to edit the cell directly, e.g. cell[2][2] += 10, which will add a vacuum in the z-direction, making a surface slab. The "Cell transformation" transforms the cell by multiplying a (3x4) matrix.

I would suggest adding instructions on how the matrix apply to the cell.

Agree. I will add a tooltip for every button.

[unkcpz]

This enables the user to edit the cell directly, e.g. cell[2][2] += 10, which will add a vacuum in the z-direction, making a surface slab.

I see, thanks for the explanation, this is interesting. But don't you think it is mind-binding to set the off-diagonal for changing the cell in this way?

Agree. I will add a tooltip for every button.

I think that is not enough, my suggestion is for each matrix you keep the matrix on the left and on the right have a paragraph for the instruction, just like

If the special symbols are hard to add, we can leave a text paragraph for future polish.

[superstar54]

keep the matrix on the left and on the right have a paragraph for the instruction, just like

Will this make too much text on the GUI? I think a detailed explanation would be better put in the documentation.

[unkcpz]

Will this make too much text on the GUI? I think a detailed explanation would be better put in the documentation.

Emm... I can't agree with this, since the structure editor widget is default hidden and only shows when the user needs to use it. Meanwhile, we don't have documentation for QeApp, the application itself needs to be explainable.

After discussing in person, I now understand what the first matrix was used for, thanks!! I am a VESTA user, so I also suggest to using the similar format for it which is the a,b,c, alpha, beta, gamma for cell setting. Like:

[yakutovicha]

@cpignedoli, could you please also have a look/test this PR? Just to make sure that it is implemented according to the user's needs.

[cpignedoli]

Great idea @superstar54 , I had requests to implement a cell editor to create surfaces but still had no time to go in this direction. Happy to see that things started.

I have a few comments from my experience with the simulation of surfaces: typically I start from a problem defined, for example, as "simulate a molecule on a stepped ( e.g. Au(11 11 12) ) gold surface

Or simulate the S1(110) surface.

What I do is 1)I start from a bulk and scale the bulk to the desired lattice parameter (will I use the exp lattice parameter, or the one compatible with my DFT settings, or, am I studying a stressed crystal,..)

2)I enter the cleavage plane in the editor (e.g. 1, 1 ,1 or 11, 11, 12) and select a thickness for the slab (this is definitely not intuitive with high indexes, you need the graphical support..), and the amount of vacuum I want to add (can also be 0 if I want to simulate a bulk in "a surface cell" (e.g. to compute projected bulk band structures) and I click the "cleavage button"

3) I adjust the thickness to the desired one

4)I "orient to standard" so that a1 coincides with x, a2 is in the xy plane (or I select a different standard if needed)

5) At this point in many cases (e.g. (111) of a cubic crystal that would produce a hexagonal cell) I redefine the lattice as a linear combination of the primitive 2D vectors to obtain for example a rectangular cell (e.g. I typically work not with the hexagonal Au(111) but with (a1 +a2 , a2-a1) that is rectangular

6)I reorient to standard

7) I replicate as desired

All of this without deriving myself the transformation matrices because for sure I would do hundreds of mistakes.

I hope my comments can be useful. We can also have a zoom call to see more examples

[superstar54]

@cpignedoli Thanks for the details. Very helpful.

1)I start from a bulk and scale the bulk to the desired lattice parameter (will I use the exp lattice parameter, or the one compatible with my DFT settings, or, am I studying a stressed crystal,..)

This PR tries to implement this (editing cell).

For the rest of the steps (building a surface slab), I think it's better to open a new PR.

I suggest using ase.build.surface to handle surface index, thickness, rectangular and so on.

[AndresOrtegaGuerrero]

@yakutovicha Should we merge this PR , so the QE app can have the supercell and cell parameters editor?

[unkcpz]

Can anyone remind me what blocks this PR? Please add a comment.

[unkcpz]

Hi @yakutovicha @cpignedoli, any update on this PR? Can we get only the supercell functionality merged and if there are more you want to add to this editor, we leave it to the future PRs?

[yakutovicha]

Hi @yakutovicha @cpignedoli, any update on this PR? Can we get only the supercell functionality merged and if there are more you want to add to this editor, we leave it to the future PRs?

We briefly discussed this with PR @cpignedoli. We will give it a look this afternoon.

[unkcpz]

thanks @cpignedoli.

@superstar54 please rebase this PR with the master branch and see if you can address the points mentioned by @cpignedoli. Thanks. As we discussed, this feature is better included in the next release.

[codecov[bot]]

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (fc0e7e1) 85.76% compared to head (a1368c3) 85.82%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #372 +/- ## ========================================== + Coverage 85.76% 85.82% +0.06% ========================================== Files 27 27 Lines 4553 4608 +55 ========================================== + Hits 3905 3955 +50 - Misses 648 653 +5 ``` | [Flag](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [python-3.10](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `85.82% <90.90%> (+0.06%)` | :arrow_up: | | [python-3.8](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `85.86% <90.90%> (+0.06%)` | :arrow_up: | | [python-3.9](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `85.86% <90.90%> (+0.06%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [tests/test\_structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-dGVzdHMvdGVzdF9zdHJ1Y3R1cmVzLnB5) | `100.00% <100.00%> (ø)` | | | [aiidalab\_widgets\_base/structures.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL3N0cnVjdHVyZXMucHk=) | `84.06% <89.36%> (+0.37%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[superstar54]

Thanks @cpignedoli, for the suggestions. I have implemented:

• [x] button to reset the matrix to 1 1 1
• [x] the matrix should be probably reset when we import a new structure
• [x] checkbox to scale atoms positions
• [x] unit test

For the other two comments:

axes.. this is a viewer problem but becomes crucial here

I agree. We should address this in another PR.

HTML help text with e.g. an example on how you get a 111 surface from a gold conventional cell (or, how do I get Au(788) )

Note, that this PR is only used to edit the cell parameters, or make supercell, not to cut the surface.

[superstar54]

Hi @yakutovicha would you like to review this, or should I proceed with merging it?

[yakutovicha]

Hi @yakutovicha would you like to review this, or should I proceed with merging it?

Let me give this PR a look today, just to be sure I didn't miss anything.

It will be quick, I hope 😄

[yakutovicha]

I agree. We should address this in another PR.

I made an issue for this #532

[yakutovicha]

In that

Note, that this PR is only used to edit the cell parameters, or make supercell, not to cut the surface.

I guess in this case we should modify the comment at the top, which says

Add the following features (https://github.com/aiidalab/aiidalab-widgets-base/issues/367, https://github.com/aiidalab/aiidalab-widgets-base/issues/365)

to prevent an accidental issue close

[superstar54]

Hi @yakutovicha , thanks for your review. I've incorporated the changes based on your suggestions.