search

TRI-AMDD / htp_md

Shared repo for trajectory analysis and infrastructure development
MIT License
13 stars 8 forks source link

Added Nernst-Einstein approximation #64

Closed arthurfl closed 1 year ago

HakyungKwon-TRI commented 1 year ago

Additionally, were you planning on adding other methods as well, or just the NE approx?

arthurfl commented 1 year ago

Looks good, but can we make sure that the computed properties 1) are part of get_all_properties in analysis.py and 2) have associated tests?

Yes, will do this week!

arthurfl commented 1 year ago

Additionally, were you planning on adding other methods as well, or just the NE approx?

Just the NE approximation - another route would be implementing the Einstein's relation, but that poses serious concerns regarding the convergence of the conductivity, especially with limited time series (~50 ns is typically far from enough for polymer electrolytes). The same would apply for the Green-Kubo relation.

arthurfl commented 1 year ago

I've updated the test data with Nernst-Einstein quantities (conductivity and transference number), as well as get_all_properties().

By the way, I've absolutely failed installing both the current version (through pip/conda) and the one of this branch (through docker), is that stable on other boxes?

HakyungKwon-TRI commented 1 year ago

@arthurfl please resolve the conflicts and merge. thank you!

arthurfl commented 1 year ago

Conflicts should be resolved now

HakyungKwon-TRI commented 1 year ago

@arthurfl it's failing some tests, one of which is @arashkhajeh-AMDD's test on population matrix?