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

Allow hcp 4-axes indices as well #390

Merged
merged 5 commits into from
Oct 22, 2021
Merged

Allow hcp 4-axes indices as well #390

merged 5 commits into from
Oct 22, 2021

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Oct 12, 2021

@prince-mathews Give this a try if it works for HCP Mg now.

@pmrv pmrv added the bug Something isn't working label Oct 12, 2021
@coveralls
Copy link

coveralls commented Oct 12, 2021

Pull Request Test Coverage Report for Build 1341981122

  • 0 of 3 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.771%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/atomistics/structure/factories/atomsk.py 0 3 0.0%
Totals Coverage Status
Change from base Build 1319432852: 0.0%
Covered Lines: 11409
Relevant Lines: 16352

💛 - Coveralls

It looks much better now. The input hkl for hcp structures should be a 3x4 matrix so I just changed that.
@prince-mathews
Copy link
Contributor

@pmrv Thank you for including the 4-axes indices. The input hkl for hcp structures should be a 3x4 matrix so I have changed it in the file.

@prince-mathews
Copy link
Contributor

Should I merge it?

@pmrv
Copy link
Contributor Author

pmrv commented Oct 12, 2021

Should I merge it?

Have you tried it on the cluster once? Because I didn't. If it works, merge it.

@prince-mathews
Copy link
Contributor

prince-mathews commented Oct 12, 2021

Should I merge it?

Have you tried it on the cluster once? Because I didn't. If it works, merge it.

How do I try it on the cluster? It's not in the master branch yet. Can I try it without it being merged?

@pmrv pmrv linked an issue Oct 14, 2021 that may be closed by this pull request
@pmrv
Copy link
Contributor Author

pmrv commented Oct 14, 2021

Atomsk on the cluster is currently out-of-date.

@pmrv
Copy link
Contributor Author

pmrv commented Oct 14, 2021

Should I merge it?

Have you tried it on the cluster once? Because I didn't. If it works, merge it.

How do I try it on the cluster? It's not in the master branch yet. Can I try it without it being merged?

I found a few bugs testing it, I'll give it a try again once the conda package is updated.

We have some older instructions here how to set up a dev environment, just replace the pyiron repo name with pyiron_atomistics.

@pmrv pmrv merged commit cf03201 into master Oct 22, 2021
@delete-merged-branch delete-merged-branch bot deleted the atomsk4 branch October 22, 2021 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atomsk Wrapper can't deal with hcp structures
3 participants