Allow control over automatic bond assignment

[andrewtarzia]

Hey all, This is working great so far. Except that my models are coarse-grained, so the bonds that come out from automatic assignment are not correct.

I have the topology information, so I can provide bonds (ideal solution), or I was think I could generate cylinder shapes for each bond. An alternative is that I could scale, or modify, the radii constrain for determining bonding information.

Any suggestions on which approach is best?

[ceriottm]

If you want to implement the machinery to provide bond information we'd be open to include it (though I'd suggest we first have a bit of discussion on how to do it so that it doesn't make other use cases more complicated or slow). We most definitely do not have the bandwidth to actively help though.

Otherwise, if you want to do this with existing infrastructure, manually defining the bonds as shapes is probably the easiest way, look at the implementation of ase_merge_pi_frames.

[andrewtarzia]

I would be very happy to contribute this feature in the future. I am not familiar with typescript, but it does seem like that would be quite a major overhaul of dataset.ts and viewer.ts to do this without breaking anything.

So, I will work with the ase_merge_pi_frames code for now, which seems to do exactly what I want with the shapes.

Thank you!

[ceriottm]

If I may make a suggestion - if you make a topology_to_shapes utility function that is fairly general and based on standard topo formats, this would be a very much welcome contribution. I'm leaving this open in case you go this way.

[Luthaf]

This would be quite nice to have. If someone wants to take a shot at it, here is what would need to happen for a full, clean integration:

• add bond information in the Structure interface, likely as Array[[number, number]], where each number is the index of one atom in the bonds
• use this bond information in the StructureViewer, using the current automatic bond assignment only if the data is not provided in the input.
• update the Python code to emit bond information when converting from user-provided structures. I don't think ASE stores the topology in the Atoms objects, so this would be a feature of other structure converters / manually added after conversion.

None of this should break other code, it should be fairly self-contained!

[andrewtarzia]

Thank you, both!

I will start with the topology_to_shape approach because it seems simpler.

Once I have that working, I can share that and we can see if @Luthaf suggestion would work nicely based on it.

Thank you again!

[ceriottm]

I think both options are useful to have so it's great to start from the easy one. Good chemiscoping.

[andrewtarzia]

Hey both,

I have managed to get bonding information written as shapes (EDIT: in the sense that an exception is not thrown....), but I am not seeing the shapes in the viewer, and I cannot figure out if there is something I am obviously doing wrong. Attached is a json file with one structure:

cs_2p3_toff_2P3.json

Because there are more bonds than atoms, I had to use the structure properties to add vectors. But then you need one entry per structure for a given shape, so I have N shapes for the number of bonds, which have a list of vectors and positions for each structure.

[ceriottm]

yes this definitely won't work, you're right. didn't think that the path integral helper has one bond per atom.
I can see two options (besides doing the "right thing" straight away and implement @Luthaf solution). the "I really want a hack" way would be to create dummy atoms on which to pin the bonds, the other (with a complexity which is half-way) is to create a shape type that is a list of vectors so it can draw many cylinders per frame. Still, it's strange that the file you created does not show any bond at all, it should work.

[andrewtarzia]

I think, considering my time frame, it makes sense to just learn something new and implement the "right thing". I am not in a rush, and I like the idea of contributing to this project.

I will submit a PR, probably in draft form with something to discuss at some point.

I agree that it is strange that that file fails to show any shape.

[Luthaf]

So #363 should fix the (two) issues related to using cylinders to display bonds.

so I have N shapes for the number of bonds, which have a list of vectors and positions for each structure.

I can see two other workarounds here: (a) you can create a single "custom" shape instead of cylinders, containing all bonds in the system (by manually building a mesh corresponding to all the cylinders) or (b) you can add multiple shapes and then add settings in your JSON file to always select them all by default, which removes the need to select them for people viewing the dataset.

[andrewtarzia]

That is great, thank you! So based on that fix, the file I sent should work (with the addition of the named shapes in settings)?

[Luthaf]

Yes, this file works with the linked PR. If you checkout the branch locally, you can run pip install . and use it from Python, or just npm start to get the website interface & load your dataset there.

[andrewtarzia]

This works a charm - I am happy to contribute the topology to shapes code as a utility once I make it more general if you are all interested. I still intend on doing the more complete method @Luthaf described but it may take some time.

EDIT:
My code currently works specifically with stk molecules (https://stk.readthedocs.io/en/stable/), but I think the interface could be generalised.

[andrewtarzia]

There is an issue in my code for the example of this.

I think the settings need to be added to the second jupyter visualisation.

[ceriottm]

@andrewtarzia I was having a look. I really like the possibility of doing custom bonds, and it's great that you have now something you can use in your project. I'd really like however (if you have the bandwitdh) not to close this yet, and try to move towards a "cleaner" solution - in particular one in which you don't have to define a separate shape per bond.
I'm thinking of defining a collection shape type where you can define a base shape and then essentially add a dimension to the arrays that define the positions so you can make a single shape type that contains all bonds.

[Luthaf]

As mentioned in #361 (comment), I think the "clean" way to do custom bonds is not to use the shapes but rather to embed the information inside the JSON as a list of pairs of indices, and then leave the rendering to 3DMol.js

This way, custom bonds will be properly integrated with automatically guessed bonds.

[andrewtarzia]

Hey both, I do agree. And now that I feel a bit more familiar with the underlying code, I would be willing to give it a go. I have some holidays coming up, so it won't be until after them. If it's ok to leave it open for now and I'll come back to it in a month or so?

[Luthaf]

Yes, this can wait!