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

Use lammps_create_atoms() #666

Merged
merged 3 commits into from Jul 4, 2022
Merged

Use lammps_create_atoms() #666

merged 3 commits into from Jul 4, 2022

Conversation

jan-janssen
Copy link
Member

No description provided.

@jan-janssen
Copy link
Member Author

@srmnitc I am not sure if we fully implemented this in pylammpsmpi do you use it in calphy?

@jan-janssen jan-janssen added the format_black reformat the code using the black standard label Jun 29, 2022
@jan-janssen
Copy link
Member Author

Be careful this is an evil pull request #666

@samwaseda
Copy link
Member

Is it a new functionality offered by LAMMPS?

@coveralls
Copy link

coveralls commented Jun 29, 2022

Pull Request Test Coverage Report for Build 2584520930

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 68.633%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/lammps/interactive.py 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
pyiron_atomistics/lammps/interactive.py 1 38.29%
Totals Coverage Status
Change from base Build 2571346632: 0.02%
Covered Lines: 11818
Relevant Lines: 17219

💛 - Coveralls

@jan-janssen
Copy link
Member Author

Is it a new functionality offered by LAMMPS?

I guess it was introduced in lammps/lammps#2310 - roughly two years ago but it was new to me.

@srmnitc
Copy link
Member

srmnitc commented Jun 29, 2022

@jan-janssen From here, it seems that scatter_atoms can update atom properties after it has been created. So we may want to keep both.

@srmnitc I am not sure if we fully implemented this in pylammpsmpi do you use it in calphy?

No its not used in calphy; the atoms are simply read in with read_data command

@jan-janssen
Copy link
Member Author

@jan-janssen From here, it seems that scatter_atoms can update atom properties after it has been created. So we may want to keep both.

Yes, we still have interactive_positions_setter() that uses scatter to modify the positions, while interactive_structure_setter() updates the whole structure
https://github.com/pyiron/pyiron_atomistics/blob/lammps_create_atoms/pyiron_atomistics/lammps/interactive.py#L83

@srmnitc
Copy link
Member

srmnitc commented Jun 29, 2022

@jan-janssen From here, it seems that scatter_atoms can update atom properties after it has been created. So we may want to keep both.

Yes, we still have interactive_positions_setter() that uses scatter to modify the positions, while interactive_structure_setter() updates the whole structure https://github.com/pyiron/pyiron_atomistics/blob/lammps_create_atoms/pyiron_atomistics/lammps/interactive.py#L83

Thanks for the clarification! Everything looks good to me.

Copy link
Member

@samwaseda samwaseda left a comment

Choose a reason for hiding this comment

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

If it works why not

@jan-janssen jan-janssen merged commit 167984d into master Jul 4, 2022
@jan-janssen jan-janssen deleted the lammps_create_atoms branch July 4, 2022 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black reformat the code using the black standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants