Skip to content

Add RDF generation - #25

Closed
ced4rtree wants to merge 3 commits into
cmelab:mainfrom
ced4rtree:rdf-for-everybody
Closed

Add RDF generation#25
ced4rtree wants to merge 3 commits into
cmelab:mainfrom
ced4rtree:rdf-for-everybody

Conversation

@ced4rtree

Copy link
Copy Markdown
Contributor

I was going to work on adding the unit test that ensures no particles are closer together than 0.9 or so in a different PR, but I can do that in this one if that's better.

@ced4rtree
ced4rtree force-pushed the rdf-for-everybody branch from 9fbe438 to b89ec4b Compare June 10, 2026 23:58
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ced4rtree
ced4rtree force-pushed the rdf-for-everybody branch from b89ec4b to b80c339 Compare June 10, 2026 23:59
@erjank
erjank self-requested a review June 11, 2026 13:40

@erjank erjank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess have a look at PR #26 and decide what we'd like to fold together or discard.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall looks good, but two small nitpicks: I don't think we want to calculate average RDFs in this case, just the last frame, and reading from a GSD that has been written out adds some complexity relative to calculating RDFs as the simulation proceeds (which could be used in our stop criteria)

import hoomd
import time
from cmeutils.sampling import is_equilibrated
import csv

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need csv?

@ced4rtree

Copy link
Copy Markdown
Contributor Author

The way this implementation is structured seems like it would be a pain to use rdf data as a stop criterion compared to #26, so I'm going to close this PR.

@ced4rtree ced4rtree closed this Jun 11, 2026
@ced4rtree
ced4rtree deleted the rdf-for-everybody branch June 11, 2026 18:30
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