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

Create plotting function for VACF #23

Merged
merged 9 commits into from Jul 12, 2023
Merged

Conversation

xhgchen
Copy link
Collaborator

@xhgchen xhgchen commented Jul 10, 2023

Towards #7

Changes made in this Pull Request:

  • Add plot_vacf() to easily plot VACFs via Matplotlib
  • Add tests to validate plot data, plot labels, and start, stop, step functionality

@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #23 (a5ad681) into main (45dd445) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@xhgchen
Copy link
Collaborator Author

xhgchen commented Jul 10, 2023

The code should work, but I am still figuring out how to write tests for it. Also want to consider and discuss whether there is a better way of going about this.

@xhgchen
Copy link
Collaborator Author

xhgchen commented Jul 11, 2023

Tests are done, but I am struggling with the type hinting for specific class instances. Any advice @hmacdope @orionarcher?

@xhgchen xhgchen marked this pull request as ready for review July 11, 2023 01:24
@@ -228,3 +229,41 @@ def _conclude_simple(self):
self.results.vacf_by_particle[lag, :] = np.mean(sum_veloc, axis=0)
# average over # particles and update results array
self.results.timeseries = self.results.vacf_by_particle.mean(axis=1)

Copy link
Member

Choose a reason for hiding this comment

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

If this takes a VACF object only it should be a method rather than standalone.

@@ -228,3 +229,41 @@ def _conclude_simple(self):
self.results.vacf_by_particle[lag, :] = np.mean(sum_veloc, axis=0)
# average over # particles and update results array
self.results.timeseries = self.results.vacf_by_particle.mean(axis=1)


def plot_vacf(vacf_obj, start=0, stop=0, step=1):
Copy link
Member

Choose a reason for hiding this comment

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

can do type hint with a string `vacf_obj: "ObJType..." but see above comment.

* Method is in class `VelocityAutocorr`
@xhgchen xhgchen requested a review from hmacdope July 11, 2023 17:49
Copy link
Member

@hmacdope hmacdope left a comment

Choose a reason for hiding this comment

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

LGTM!

@xhgchen xhgchen merged commit bebcfa1 into MDAnalysis:main Jul 12, 2023
24 checks passed
@xhgchen xhgchen deleted the plot-vacf branch July 12, 2023 02:48
@xhgchen xhgchen restored the plot-vacf branch July 12, 2023 02:49
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.

None yet

2 participants