feat: structure object to ASE structure objects adapter #127

Activity

1. TingwenZhang commented on May 23, 2025

   

   TingwenZhang

   on May 23, 2025

   Contributor

   More actions

   I can attempt on this issue. However, this issue may take me a month. Is there a deadline on this issue?
2. sbillinge commented on May 24, 2025

   

   sbillinge

   on May 24, 2025

   ContributorAuthor

   More actions

   @TingwenZhang I suggest that you work on one or two of the other issues first to practice the workflow, then we can switch to this one (which is the main event and can take longer). we have a particular workflow where we discuss first behavior, then design, then tests then the code.
3. TingwenZhang commented on May 24, 2025

   

   TingwenZhang

   on May 24, 2025

   Contributor

   More actions

   Got it.

   👍React with 👍1Reacted by Simon Billinge
4. sbillinge

   added this to the 3.4.0-rc.0 release milestone on Jun 27, 2025
5. sbillinge

   modified the milestones: 3.4.0-rc.0 release, 3.5.0 release on Mar 14, 2026
6. cadenmyers13 commented on Apr 2, 2026

   

   cadenmyers13

   on Apr 2, 2026

   Contributor

   More actions

   Moving the discussion on this PR to here. Below is some of that discussion:

   from @cadenmyers13

   Here's the UCs i thought of and is what guided the code on this PR. Only option 2 is included here. Option 1 would be on a different PR. Both 1a and 1b would be things that would need to be implemented in ase, but i included them here for completion:

   User wants to convert diffpy --> ase
   a. User calls a method in diffpy.structure.Structure and passes diffpy obj to convert diffpy --> ASE, or
   b. User calls a method in ASE.Atoms and passes diffpy obj to convert a diffpy --> ASE

   User want to convert ase --> diffpy
   a. User calls a method in diffpy.structure.Structure and passes ase obj to convert diffpy --> ASE, or
   b. User calls a method in ASE.Atoms and passes ase obj to convert a diffpy --> ASE

   Any other use cases you could think of?

   Also, we might be able to get away with this without adding it as a dependency. The only use for it is an isinstance check that raises an error, but we could probably remove this and the user would get an error downstream generated by something else. Presumably, if you're trying to convert a diffpy object to ase you already have ase installed in your env. Idk how removing this will go over in testing though. We could just add ase to tests.txt, right?

   from @sbillinge

   The main question is serialization. I think that if it is possible to serialize ASE objects we can read them without an ASE dependency. We can also have code to write them.

   Your UCs are good for some kind of high throughout pipeline where ASE is generating large numbers of objects that are being passed to Diffpy and serialization would be a serious performance bottleneck. However I do see the use and convenience. It occured to me that we could make an ASE pack so we don't pick up a new dependency in the core.

   I think that thinking of actual UCs could be good. The one highest on my list would be to take an MD simulation and Compute the PDF. Another would be to support something like Soham's cluster mining. We should maybe check how he did it as he may already have some code. I would guess that the code is on gitlab

   For these UCs do you know if it is Atoms objects or Some other kind of ASE objects that is passed?
7. cadenmyers13 commented on Apr 20, 2026

   

   cadenmyers13

   on Apr 20, 2026

   Last edited by cadenmyers13

   Contributor

   More actions

   Your UCs are good for some kind of high throughout pipeline where ASE is generating large numbers of objects that are being passed to Diffpy and serialization would be a serious performance bottleneck. However I do see the use and convenience. It occured to me that we could make an ASE pack so we don't pick up a new dependency in the core.

   @sbillinge ChatGPT recommends doing a lazy import and not make it a dependency and not to serialize. It says this is common practice. Something like this,

       try:
           from ase import Atoms
       except ImportError:
           raise ImportError("ASE is required for conversion to ASE objects.")

   For these UCs do you know if it is Atoms objects or Some other kind of ASE objects that is passed?

   ase.Atoms is the analogous object to diffpy.structure.Structure.

   I agree with you about the MD workflow. I don't think that would be too difficult to implement with a lazy import. The UC would be like:

   1. User run MD
   2. they get an ase.Atoms object out
   3. they want to compute a PDF of it
   4. they pass the ase.Atoms object to diffpy.structure.Structure.convert_ase_to_diffpy()
   5. the analogous diffpy.structure object is instantiated with all necessary information.
   6. User can then easily calculate pdf from this object

   TLDR: Let's get started on an ase to diffpy.structure adapter using a lazy import. what do you think?
8. sbillinge

   modified the milestones: 3.5.0 release, v3.6.0 release milestone on Aug 9, 2026