materialsproject / crystaltoolkit

Crystal Toolkit is a framework for building web apps for materials science and is currently powering the new Materials Project website.
https://docs.crystaltoolkit.org
Other
135 stars 57 forks source link

Most examples apps broken #361

Closed janosh closed 9 months ago

janosh commented 11 months ago

Total example apps:

ls crystal_toolkit/apps/examples/*.py | wc -l
>>> 23  # -2 for utils and __init__

Found 6 broken out of 8 tested

  1. apps/examples/pourbaix.py
  2. apps/examples/structure.py
  3. 💀 apps/examples/phase_diagram.py
  4. 💀 apps/examples/relaxation_trajectory.py
  5. 💀 apps/examples/structure_magnetic.py
  6. 💀 apps/examples/transformations_minimal.py
  7. 💀 apps/examples/transformations.py
  8. 💀 apps/examples/write_structure_screenshot_to_file.py

Below are app screenshots.

apps/examples/pourbaix.py

pourbaix

apps/examples/structure.py

structure

💀 apps/examples/phase_diagram.py

phase_diagram

💀 apps/examples/relaxation_trajectory.py

relaxation_trajectory

💀 apps/examples/structure_magnetic.py

structure_magnetic

💀 apps/examples/transformations_minimal.py

transformations_minimal

💀 apps/examples/transformations.py

transformations

💀 apps/examples/write_structure_screenshot_to_file.py

write_structure_screenshot_to_file

mkhorton commented 11 months ago

Thanks for this summary. I think next step here is to make sure these are working, and add to automated testing (e.g., making sure they run without console errors).

Quick impressions:

tsmathis commented 11 months ago

Re: the screenshot generation script: There previously was an imageRequest property within the CrystalToolkitScene component in mp-react-components. At some point in the last two years that property and the associated hooks were removed from mp-react-components, I haven't found the exact commit this occurred at yet so I'm not sure what the reason was for removing it.

I had to re-introduce the imageRequest prop for a recent ad hoc image generation script for new structures. So it could be worth a deeper look.

mkhorton commented 11 months ago

Sorry @tsmathis, sounds frustrating!

I can offer some background on those props for context. Basically, there are two ways of handling a screenshot or 3D model export. Both of these ways can, in principle, be used together:

  1. Trigger the download completely client-side
    • Pros: fast
    • Cons: server cannot retrieve downloaded data
  2. Encode the image/file as a prop, b64-encoded
    • Pros: server can then modify this image (for example, we might want to add a legend, or generate screenshots automatically and upload these to a database)
    • Cons: I felt uneasy about putting a relatively "large" string into a prop

I don't remember exactly, but I think potentially 1. was removed in favor of 2.? I think the capability was always there somehow, it was just a question of whether it was done client-side or server-side.

Regardless of outcome, the ultimate aim was to add both some legend tiles + a credit string to the image via Pillow, as a polite nudge to people to remember to credit the source of the image when it's re-used. This was on the backlog.