Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access and Stream Trajectories with MDAnalysis #287

Merged
merged 55 commits into from
Sep 24, 2023

Conversation

yuxuanzhuang
Copy link
Contributor

@yuxuanzhuang yuxuanzhuang commented Aug 10, 2023

Fix #292

Summary of Changes:

  • Registration of handlers to update trajectory frames and save the mda session now occurs at the outset.
  • The initialization of the previous import subroutine in Blender AI incorporates the use_old_import flag (I'm open to changing this to a more appropriate option or making it the default setting).
  • Code responsible for generating the MDAnalysisSession and creating objects has been relocated to mda.py.
  • The utilization of the AtomGroupInBlender class facilitates the conversion and retrieval of topology/attributes from AtomGroups to Blender.
  • 'molecule' tag is added to all molecular objects in Blender. This helps to decide whether an mda session should be loaded when a blend file is loaded.

@yuxuanzhuang yuxuanzhuang marked this pull request as draft August 10, 2023 11:50
@yuxuanzhuang yuxuanzhuang changed the title Access and Stream Trajectory with MDAnalysis Access and Stream Trajectories with MDAnalysis Aug 10, 2023
@BradyAJohnston
Copy link
Owner

Thanks for the PR! I have been wanting to try and attempt this for quite a while, and it seems like you have mostly nailed a bunch of it straight away.

I have had a bit of a play around with it and it seems to work really well initially. If I save and load a file I lose the connection to the trajectory and it stops updating. It also occasionally loses the connection to the data / universe while I am playing around inside of the scene, but I am unsure what specifically I am doing that is causing it.

Potential fixes for this (and for your note about multiple universes) is that the universe information could be stored on the object itself. I currently store some data on the objects (like chain IDs etc) so we could potentially store that data there instead of on the scene level.

I think having the option on import for the user to either stream the trajectory, or load in to memory, would be good as well. That way you can have a self-contained .blend file instead of having to worry about connections to other files if you don't want to. That would also still enable geometry-nodes playback through that import method.

I'm away for the weekend so I won't be able to give feedback on specific code until next week, but this is working really well already.

@BradyAJohnston
Copy link
Owner

Additionally, if you have other suggestions for refactoring the md.py script, please do suggest them! I wrote the majority of it a year or so ago (very early to python then), and I haven't revisited it in a while so I would gladly accept suggestions and improvements on the larger codebase.

@yuxuanzhuang
Copy link
Contributor Author

I currently store some data on the objects

It seems that only simple data types can be saved as attributes in bpy.types.Object and the Universe is not simple!

I think one possible way to rejuvenate the animation is by dumping/serializing the Universe into a string and deserializing it during the reloading process. Besides, I think most Python-level stuff will not be saved into .blend files e.g. functions inside frame_change_post so it also needs to be reappended during reloading. I just need to have a look at how it can be achieved :) and please let me know if you have any insights!

@yuxuanzhuang
Copy link
Contributor Author

Still WIP but now the whole session can be reloaded. Try the notebook with (https://github.com/cheng-chi/blender_notebook) here https://gist.github.com/yuxuanzhuang/25aa5a1588c7e19efbfc1b7275718b58

It also occasionally loses the connection to the data / universe while I am playing around inside of the scene, but I am unsure what specifically I am doing that is causing it.

I don't seem to have this issue during the session but I also haven't played around with anything inside the geometric nodes yet...

@BradyAJohnston
Copy link
Owner

BradyAJohnston commented Aug 15, 2023

I just had a play around and the saving worked well, the same with the adding of multiple objects. Even on testing on my old 2017 MacBook the streaming of trajectories works really, really well.

The connection was stable for me while scanning through even 50,000 frame trajectories without any issue. I interacted with the node trees for a while and it was fine, then I changed something in the node tree and the connection was lost. unsure exactly what I did that caused the issue, but will have to try and poke around and find out what is causing the issue, otherwise I am really liking this implementation and it's working really well.

We might want to try and brainstorm some more design ideas around how to better interact with Blender through more convenience functions and classes. I don't really have much experience with classes, having the object be a class with a bunch of useful methods like changing the style or other common operations would likely help with interactions from the notebook itself. The same with interacting with the Blender scene in general, like setting cycles render engine etc, something to help with with the scene is likely a good idea, but I wouldn't know what would be best in terms of implementation, out of my depth in that respect.

Very excited by the prospect of this though, it has already turned Blender into a pretty great molecular viewer. Amazing what you've managed to do so quickly.

Tasks that I'll do:

  • Clean up a bunch of the messages. This was fine when it printed to Blender's console but messhy in a notebook
  • Improve some default styles, better 'single node' usage that will enable interactions via scripting better

@yuxuanzhuang
Copy link
Contributor Author

I managed to reproduce the disconnection (error msg: ReferenceError: StructRNA of type Object has been removed) by simply undoing anything. It's probably due to the memory address that holds the pointer to the data changing every time an undo is performed. The latest commit should have fixed it.

Copy link
Owner

@BradyAJohnston BradyAJohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I've said I'm not super familiar with a lot of class-based stuff (I've mostly come from R programming background) but it all looks pretty good.

It seems like already as is this works REALLY well.

A potential gotcha would be when deleting an object. Currently if I delete an object I then get errors as it tries to update the object. There might be some kind of on_delete function we can register that cleans up objects when deleted.

MolecularNodes/mda.py Outdated Show resolved Hide resolved
MolecularNodes/mda.py Outdated Show resolved Hide resolved
@@ -45,6 +45,7 @@ def create_object(name: str, collection: bpy.types.Collection, locations, bonds=
mol_mesh = bpy.data.meshes.new(name)
mol_mesh.from_pydata(locations, bonds, faces=[])
mol_object = bpy.data.objects.new(name, mol_mesh)
mol_object['type'] = 'molecule'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I have been thinking about for a while. I haven't come up with any kind of scheme, but the different objects that are added to Blender though Molecular Nodes currently aren't tagged with any identifying information. Tagging with molecule and maybe other info is a good idea. Open to suggestions for naming schemes.

MolecularNodes/md.py Outdated Show resolved Hide resolved
@yuxuanzhuang yuxuanzhuang marked this pull request as ready for review August 21, 2023 09:31
@BradyAJohnston
Copy link
Owner

I can't for the life of me figure out why the docs keep failing to auto-build. I assume it might be just a permissions thing for this PR, but they are building fine for me and I will be able to deploy them myself later.

I've added a quick overview in the 03_moelcular_dynamics.md file. If you want to add more detail, please do! I have just been uploading screenshots & videos to imgur and then linking to them, which seems to be the best stratergy.

@yuxuanzhuang
Copy link
Contributor Author

I can't for the life of me figure out why the docs keep failing to auto-build. I assume it might be just a permissions thing for this PR, but they are building fine for me and I will be able to deploy them myself later.

Maybe try this? bobheadxi/deployments#78 (comment)

@BradyAJohnston
Copy link
Owner

I generated a new PAT (is that the right one??) and added it as a secret to the repo for GHA, but it seems to get null when it access the token so I am beyond stumped at the moment. Going to bed so might figure something out when I wake up or if you can figure something out that would be amazing.

Like I said, not the end of the world because it will work outside of this PR, just failing for this PR.

@yuxuanzhuang
Copy link
Contributor Author

No problem! Just revoked all the edits on the yml file.

test new token

without token

set deployments to v1

revoke edits on docs.yml

tweak permissions

tweak persmissions

tweak PAT

tweak PAT

tweak PAT
@BradyAJohnston
Copy link
Owner

I think this is ready to merge in to main. It seems to be fairly robust, and doesn't remove any features that were already there. There are lots of exciting things to build on top of it, but think this is good to get into main and we can create a release and maybe a new branch for the MDA hackathon.

@BradyAJohnston BradyAJohnston merged commit 41b3f18 into BradyAJohnston:main Sep 24, 2023
6 of 7 checks passed
@yuxuanzhuang
Copy link
Contributor Author

Thanks!!!

@BradyAJohnston
Copy link
Owner

@yuxuanzhuang the thanks is all for you! thanks so much for working on this. it's going to enable a lot of very cool stuff in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Access and Stream Trajectories with MDAnalysis inside MolecularNode
2 participants