Dislocation simplification & generalisation

[thomas-rocke]

Main Features:

• Simplification of dislocation class interfaces, leveraging dimensionless properties
• Removal of near-identical init methods, leading to improved maintainability
• Generalisation of interface to support arbitrary cubic structures

[pgrigorev]

Thank you for your work, I like the idea of simplification and generalisation! It looks like there is plenty of changes. I will have a closer look this week.

[jameskermode]

Looks good to me with the small comments noted in my review.

@pgrigorev please take a look as well to make sure none of this will impact negatively on your use of this functionality.

After that I'll be happy to merge.

[thomas-rocke]

Waiting on #188 to be merged, in order to resolve plasticity documentation between that PR and this one.

[thomas-rocke]

Hi @pgrigorev , I tried to use your show_dislocation function, and although I think it's extremely useful for the single-species plots, the colouring of e.g. the diamond lattice region makes the chemical complexity less visible, and therefore makes the plots harder to read. Ideally, I'd like to keep using interactive_view, although it is not showing up when I build the docs locally - is this the issue you were talking about?

[pgrigorev]

Hi @pgrigorev , I tried to use your show_dislocation function, and although I think it's extremely useful for the single-species plots, the colouring of e.g. the diamond lattice region makes the chemical complexity less visible, and therefore makes the plots harder to read. Ideally, I'd like to keep using interactive_view, although it is not showing up when I build the docs locally - is this the issue you were talking about?

The issue I was mentioning was related to dependencies in github actions and was solved in #199, you can see correctly rendered notebook here now. I think to fix your problem you should add import nglview in one of the first cells of th notebook same way it is is done in the cylinder_configurations.ipynb.

I see your point. I would suggest a compromise. We can just add a flag to show_dislocation to add CNA coloring. In this way you can keep coloring of the atoms according to chemical species, but the tooltip will show information about identified structure and you will be able to add dislocations lines. I can do it for you. show_dislocation is really a better version of interactive_view. It does not make sense to keep both.

I also noticed that in your branch the cylinder_configurations.ipynb has some cells that I deleted in the final version (cells with Vitek plots for examples). Merging does not very well with ipython notebooks. I am afraid you will have to add your cells on top of the final version of the notebook to be sure nothing is lost or broken.

[thomas-rocke]

I'd be happy to use show_dislocation without the CNA colouring, so this sounds like a good compromise. Thanks for the heads up about the deleted cells as well - I'll work on trying to unpick those merge errors today.

[thomas-rocke]

Changes made to include new features of show_dislocation, resulting in some really nice plots! I think I made a Git mistake in doing git checkout -- rather than a git merge -- to fastforward visualisation.py to include changed from @pgrigorev (I really did not want to have to deal with another merge conflict on the jupyter notebook, the last one took ages to fix!). Should be ready for review now, @jameskermode

[jameskermode]

Happy to merge subject to those two minor comments and docs changes

[pgrigorev]

The final version of rendered documentation looks fantastic! I like zoomed in visualisations of alpha and beta dislocations. Great work, @thomas-rocke. @jameskermode, did we add something to the paper mentioning new functionalities. I added a comment in the review issue to tell them we updated the docs. We could mention the new feature there as well now.

[jameskermode]

Thanks both! Yes, Tom added a remark to the paper too.