Improve TEMDiffractionComponent to use py4DSTEM

[sezelt]

Following #259, update TEMDiffractionComponent to use py4DSTEM for diffraction calculations.

Tasks (adapted from the original issue):

• Add py4DSTEM to (optional) project requirements and ensure test suite still passes
  • The py4DSTEM package currently has a hard dependency on PyQt5, which is fine on my desktop but breaks install on an ARM Mac and may(?) not work on the MP servers. PyQt5 is not needed for the functionality we use here, so if need be we can make it optional in upstream py4DSTEM.
• Enhance TEMDiffractionComponent to use py4DSTEM
  • A skeleton of how py4DSTEM will be integrated is in place, but not yet functional.
  • Some smart caching of structure factors may be needed for performance reasons. Alternatively, this can be solved by breaking up the callbacks for different options so that only the necessary inputs are recomputed when the user interacts.
• A simple example app added to crystal_toolkit.apps.examles so that this can be incorporated into the test suite and documentation
  • Add necessary options to the interface
  • Generate plotly graphs from py4DSTEM's PointList datastructure for diffraction patterns

[sezelt]

I had to rebuild the poetry.lock file to include py4DSTEM, and it seems that a small typo in mp-api is corrupting the poetry.lock, preventing the build test from getting very far, so I made a (+ 1 character) PR to mp-api to mend that: materialsproject/api#697

[mkhorton]

Just having a read over this PR, it looks excellent!

Announcement to follow, but I will be giving all collaborators (e.g., paper co-authors) permissions to merge their own PRs, to help make the project feel more collaborative and reduce delays -- as project maintainer I'll still keep an eye on which PRs are being merged and review major changes before release.

That said, since this is your first PR, I will still be glad to do a full review for this if helpful!

[sezelt]

@mkhorton I am happy with how the TEM component is looking now, so I took this out of draft status, and I think it is ready for review. At the moment the build seems to be failing for reasons I don't understand, but which also don't seem to be related to the TEM component, so I'll need some help on that.

[mkhorton]

Hi @sezelt, thanks for your patience while this sat in review.

I'm happy to merge (the merge conflicts are minor and I can resolve these). However, the only issue I'm having is with py4DSTEM installation -- installation via pip doesn't work for me, due to the PyQt5 dep, and in general the py4DSTEM dependencies seem very heavy relative to the functionality being used here. Do you have any thoughts on this?

Attaching screenshot for posterity. Functionality-wise this seems perfect, and very well integrated into the existing code-base, so thank you!

[mkhorton]

Ah, I don't have permissions to resolve this merge conflict myself.

Essentially, the merge conflicts are trivial, you should use pull-request.yml, __init__.py, poetry.lock as-is from main (poetry.lock was deleted, due to a change in build system), use diffraction.py as-is from your PR, and the only file that needs to be resolved manually is pyproject.toml, and for that you can use the version from main except adding the tem-diff = ["py4DSTEM"] line -- let me know if you have any questions.

[mkhorton]

Once the merge conflict is resolved, please go ahead and merge! As a contributor, I've given you write access to this repository. The PR is definitely ready 👍

In terms for getting this deployed on Materials Project, I'll have to discuss with @tschaume et al. about the py4DSTEM requirement, and see if it causes any issues on our servers.

[sezelt]

Awesome! I think I got everything right in fixing the merge conflicts, so I'm going to go ahead and merge this in now.

[mkhorton]

Thanks for the merge @sezelt!