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 RDFs calculation based on species #338

Merged
merged 6 commits into from
Nov 14, 2024
Merged

Add RDFs calculation based on species #338

merged 6 commits into from
Nov 14, 2024

Conversation

AstyLavrinenko
Copy link
Contributor

Currently, the RDFs can be calculated for floating species, requiring sites data or transition information. Could you please additionally implement a version that calculates RDFs between specified species without the need for transition data. I have provided the code we use for this calculation, which includes parallelization.

Currently, the RDFs can be calculated for floating species, requiring sites data or transition information. Could you please additionally implement a version that calculates RDFs between specified species without the need for transition data. I have provided the code we use for this calculation, which includes parallelization.
@stefsmeets
Copy link
Contributor

stefsmeets commented Nov 11, 2024

Hi @AstyLavrinenko thanks for this, I refactored the code a bit. I think I'm going to add it as a plot function to Trajectory:

trajectory.plot_rdf_between_species(...)

Would that work for you?

Todo

  • Add method on trajectory
  • Move hist code to plots/matplotlib
  • Add plotly version
  • Add integration test

@stefsmeets
Copy link
Contributor

Let me know if you are happy with this!

@stefsmeets stefsmeets merged commit 6aab085 into main Nov 14, 2024
3 of 4 checks passed
@stefsmeets stefsmeets deleted the RDFs branch November 14, 2024 09:03
@AstyLavrinenko
Copy link
Contributor Author

Hi @stefsmeets thanks a lot!
The only concern is that raw data is not easily accessible now, and it's nice to have such opportunity as sometimes we need to visualize several RDFs at one plot. Can we maybe move _get_radial_distribution_between_species function as a Trajectory function?

@stefsmeets
Copy link
Contributor

Yeah, makes sense. I came up with a better way to organize the code, I will make a PR soon.

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.

2 participants