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

Add nematic_order.py #5

Merged
merged 15 commits into from
Sep 18, 2024
Merged

Add nematic_order.py #5

merged 15 commits into from
Sep 18, 2024

Conversation

jbieri
Copy link
Collaborator

@jbieri jbieri commented Sep 12, 2024

Removed the Problem .ipynbs and added a working python file.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

This is a good start! I gave @StephMcCallum some clues about the correct way to iterate through the bonds in a GSD file, you two should work together on what needs to be changed here (this implementation isn't quite right).

preliminary-work/nematic_order.py Outdated Show resolved Hide resolved
preliminary-work/nematic_order.py Outdated Show resolved Hide resolved
preliminary-work/nematic_order.py Outdated Show resolved Hide resolved
preliminary-work/nematic_order.py Outdated Show resolved Hide resolved
@chrisjonesBSU
Copy link
Member

The recent commit seems to fix how bond vectors are calculated. I left a couple more about moving a couple of things inside the first for loop.

For each snapshot, we need a new nematic order measurement. This means we'll have to create a new freud.order.Nematic() object, and create a new list of bond vectors for each snapshot.

The step size to use when iterating through start:stop
"""
nm_orders = []
directors = [] #adding temporary directors list to export directors generated from nematic function
Copy link
Member

Choose a reason for hiding this comment

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

I like that we are also returning the director, so let's keep it, rather than it being temporary.

Copy link
Member

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

Looks good to me. @StephMcCallum you can merge this whenever

@StephMcCallum StephMcCallum merged commit e4a7d7d into main Sep 18, 2024
@jbieri jbieri deleted the add-nematic-order branch September 19, 2024 16:15
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.

4 participants