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

Post equilibrium analysis #43

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Post equilibrium analysis #43

wants to merge 15 commits into from

Conversation

gwenwhite
Copy link
Collaborator

@gwenwhite gwenwhite commented Apr 12, 2023

This is a pull request working on step 3 of issue #40. So far I have just added the RDF code to the project.py file but it still needs work and testing. Just wanted to get a PR started.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@gwenwhite gwenwhite changed the title Post equilibrium Post equilibrium analysis May 8, 2023
@@ -0,0 +1,10 @@
344.14088824424385,1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

let's add .txt to the .gitignore and remove this file.

Comment on lines +148 to +149
peakx = max(x)
peaky = max(y)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think first and second peaks are more information rich in rdf. The max of bin_centers is always going to be the last value in the list (largest distance) and max of rdf doesn't give us a correct estimation of the first peak.
Previously I used scipy.signal.find_peaks from scipy package to find the peak.
@chrisjonesBSU Feel free to correct me if I'm wrong.

Copy link
Member

@chrisjonesBSU chrisjonesBSU May 9, 2023

Choose a reason for hiding this comment

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

I agree. We should use some kind of peak finding algorithm or tool. RDFs often have more than 1 peak, so just finding the max y value may not provide enough information.

import numpy as np
os.makedirs(os.path.join(job.ws, "analysis/rdf"))
gsdfile = job.fn('trajectory_1.gsd')
rdf = rdf(gsdfile, start=-30)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please apply this change to the rdf function in utils.py

rdf.compute((box,points), reset=False)

The default value for reset is True, which means if you are computing rdf from multiple snapshots, freud doesn't accumulate the computated values from different snapshots. When reset is True, the result rdf is only from last snapshot.

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.

3 participants