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

Fix interactions #107

Merged
merged 4 commits into from
May 16, 2024
Merged

Fix interactions #107

merged 4 commits into from
May 16, 2024

Conversation

JFRudzinski
Copy link
Collaborator

No description provided.

@JFRudzinski
Copy link
Collaborator Author

@ladinesa I did not find any case with inhomogeneous interactions. I simply removed your fix and everything works fine.

In fe_test, there are interactions that don't have atom_indices or atom_labels at all, but that should be fine, they are just describing the general force field.

Can you have a look again? If you find an example with inhomogeneous interactions just point me to it so I can take a look and try to come up with a fix.

@JFRudzinski
Copy link
Collaborator Author

@ladinesa I made a fix in parse_interactions() in mdanalysis.py that avoids this lammps situation with the inhomogeneous atom_labels. I still did not see a case with inhomogeneous indices. Hopefully this is enough for now.

You will also notice that I remove some storage of (incorrect) parameters. When we work on the new schema and parser plugins, we will do a better job grabbing some of the interaction parameter information by parsing the raw files directly (MDAnalysis does not robustly access this info)

Let me know what you think and if you see any other potential issues.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9109889242

Details

  • 9 of 11 (81.82%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 88.168%

Changes Missing Coverage Covered Lines Changed/Added Lines %
atomisticparsers/utils/mdanalysis.py 4 6 66.67%
Totals Coverage Status
Change from base Build 8830505722: -0.02%
Covered Lines: 6237
Relevant Lines: 7074

💛 - Coveralls

@ladinesa
Copy link
Collaborator

Hi @JFRudzinski I just run the tests and I did not find any more errors. Please merge.

@JFRudzinski JFRudzinski merged commit b117c41 into fix-quantity-types May 16, 2024
3 checks passed
@JFRudzinski JFRudzinski deleted the fix-interactions branch May 24, 2024 09:07
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.

3 participants