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

[WIP] Fran graphs #518

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

[WIP] Fran graphs #518

wants to merge 10 commits into from

Conversation

Fratorhe
Copy link
Collaborator

@Fratorhe Fratorhe commented Apr 21, 2022

This PR should not be merged (YET).
I have a few questions and I think this is the easiest way to assess them.

I have implemented calculators for the search of neighbors per atom, and for the rings (initial development by Hamish).
The current implementation does not use TF (only numpy and scipy).

Questions:

  • How do I create a specific subject in the database? currently this is done with self.queue_data(data=dict_neighbors_result, subjects=["System"]). But it would be nicer to have appropriate names (eg. rings or neighbors)
    • You define series result keys and result key names in the init which is stored in the DB
  • I do not know how to save the timestep in the database.
    • self.time = self._handle_tau_values() * self.experiment.units["time"]
  • The bokeh plot appears as a subplot in the case of having more than one line and I do not know how to fix this...I guess it is creating another figure somewhere. Maybe in the calculator class?
    • Create your own plot_data method, see green_kubo_self_diffusion for a good example
  • Later I plan to restructure the code and create a graph parent class, but first I wanted to have them separated for development.

Summary of additions and changes

  • FindNeighbors. This is simple but useful to assess for example which kind of hybridization of carbon we have in the system and its evolution.
  • FindRings. This computes the number of rings in the structure.

How to test this pull request

Functional tests using a fullerene have been implemented. Further tests could be implemented using DataHub if needed.
The lammpstraj file added so far is very small.

In examples, I included as well a C59 structure (basically removing an atom from the C60).

@Fratorhe Fratorhe requested a review from SamTov April 21, 2022 10:18
@SamTov SamTov changed the title Fran graphs [WIP] Fran graphs Apr 21, 2022
@SamTov SamTov added the draft label Apr 21, 2022
@SamTov SamTov marked this pull request as draft April 21, 2022 13:06
@SamTov
Copy link
Member

SamTov commented Apr 21, 2022

@Fratorhe Maybe we can have a call next week or so and go over it together with the molecule mapping stuff somewhere in mind as it looks like there are some nice crossovers that we can take advantage of.

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

Successfully merging this pull request may close these issues.

2 participants