-
Notifications
You must be signed in to change notification settings - Fork 20
Merging updates to LAMMPS agent into Main #152
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
base: main
Are you sure you want to change the base?
Conversation
…ose potentials only, i.e. without actually running LAMMPS
…d back to generic lmp_mpi
…zation/visualization of results
Updating and handling conflicts. Co-authored-by: Somasundaram-Rahul
I forgot to merge changes that had been pushed to the repo since my last push. This should be resolved and up to date with current version.
luiarthur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @Somasundaram-Rahul! This is great.
I have a few suggestions.
Would adding a test to tests/ be possible? I think your examples might be sufficient. The test should ideally be fast and can be crude. Just want to make we know if/when future changes will break this code.
|
|
||
| ## Dependencies | ||
|
|
||
| The main dependency is the [LAMMPS](https://www.lammps.org) code that needs to be installed separately. LAMMPS is a classical molecular dynamics code developed by Sandia National Laboratories. Installation instructions can be found [here](https://docs.lammps.org/Install.html). On macOS and linux systems, the simplest way to install LAMMPS is often via [Conda](https://anaconda.org/channels/conda-forge/packages/lammps/overview), in the same conda environment where `ursa` is installed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The main dependency is the [LAMMPS](https://www.lammps.org) code that needs to be installed separately. LAMMPS is a classical molecular dynamics code developed by Sandia National Laboratories. Installation instructions can be found [here](https://docs.lammps.org/Install.html). On macOS and linux systems, the simplest way to install LAMMPS is often via [Conda](https://anaconda.org/channels/conda-forge/packages/lammps/overview), in the same conda environment where `ursa` is installed. | |
| The main dependency is the [LAMMPS](https://www.lammps.org) code that needs to be separately installed. LAMMPS is a classical molecular dynamics code developed by Sandia National Laboratories. Installation instructions can be found [here](https://docs.lammps.org/Install.html). On MacOS and Linux systems, the simplest way to install LAMMPS is often via [Conda](https://anaconda.org/channels/conda-forge/packages/lammps/overview), in the same conda environment where `ursa` is installed. |
|
|
||
| The main dependency is the [LAMMPS](https://www.lammps.org) code that needs to be installed separately. LAMMPS is a classical molecular dynamics code developed by Sandia National Laboratories. Installation instructions can be found [here](https://docs.lammps.org/Install.html). On macOS and linux systems, the simplest way to install LAMMPS is often via [Conda](https://anaconda.org/channels/conda-forge/packages/lammps/overview), in the same conda environment where `ursa` is installed. | ||
|
|
||
| Two additional dependencies, that are not installed along with `ursa`, are `atomman` and `trafilatura`. These can be installed via `pip install atomman` and `pip install trafilatura`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Two additional dependencies, that are not installed along with `ursa`, are `atomman` and `trafilatura`. These can be installed via `pip install atomman` and `pip install trafilatura`. | |
| Two additional dependencies, that are not installed along with `ursa`, are `atomman` and `trafilatura`. These can be installed via `pip install atomman trafilatura`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it looks like trafilatura is in the dependencies. So the only other dependency is atomman. @mikegros does it make sense to remove trafilatura from dependencies and remove the lammps group; and then add trafilatura and atomman as optional dependencies? I can make the change if wanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the html parsing utility uses trafilatura as well, so it's required more broadly. This error should really only mention atomman
| summarize_results=True, | ||
| ) | ||
|
|
||
| with open("eos_template.txt", "r") as file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this file look like?
| ) | ||
|
|
||
| if final_lammps_state.get("run_returncode") == 0: | ||
| print("\nLAMMPS Workflow completed successfully. Exiting.....") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using console.print for consistency.
|
|
||
| Summarize the contents of this file in a markdown document. Include a plot, if relevent. | ||
| """ | ||
| simulation_task = "Carry out a LAMMPS simulation of the high entropy alloy Co-Cr-Fe-Mn-Ni to determine its stiffness tensor." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't count, but if this is longer than 80 lines, split into two lines. e.g.,
simulation_task = (
"Carry out a LAMMPS ..."
"its stiffness tensor."
)
This PR merges several updates to the LAMMPS agent into the Main branch. In summary, they are: