Add Wrapper for Atomsk Structure Creation #260

pmrv · 2021-06-29T08:04:03Z

It turned out to be quite simple, so I've added it here instead of a new repo. I'll brush up the docstrings a bit and add examples and tests I got from @zhuochengXIE, but the main funtionality is there.

samwaseda · 2021-06-29T08:05:53Z

Nice! Is there a way to include dislocations? doesn't have to be this PR.

coveralls · 2021-06-29T08:13:11Z

Pull Request Test Coverage Report for Build 1237696022

33 of 74 (44.59%) changed or added relevant lines in 2 files are covered.
712 unchanged lines in 14 files lost coverage.
Overall coverage increased (+0.3%) to 68.184%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pyiron_atomistics/atomistics/structure/factory.py	3	7	42.86%
pyiron_atomistics/atomistics/structure/factories/atomsk.py	30	67	44.78%

Files with Coverage Reduction	New Missed Lines	%
pyiron_atomistics/atomistics/structure/analyse.py	2	98.9%
pyiron_atomistics/lammps/units.py	2	97.14%
pyiron_atomistics/atomistics/structure/has_structure.py	5	85.0%
pyiron_atomistics/atomistics/job/structurecontainer.py	6	77.22%
pyiron_atomistics/vasp/outcar.py	7	90.34%
pyiron_atomistics/atomistics/structure/neighbors.py	14	97.01%
pyiron_atomistics/vasp/interactive.py	20	42.11%
pyiron_atomistics/table/funct.py	21	59.48%
pyiron_atomistics/atomistics/structure/periodic_table.py	26	83.5%
pyiron_atomistics/atomistics/master/murnaghan.py	32	72.15%

Totals
Change from base Build 979303097:	0.3%
Covered Lines:	11116
Relevant Lines:	16303

💛 - Coveralls

pmrv · 2021-06-29T08:13:18Z

Nice! Is there a way to include dislocations? doesn't have to be this PR.

Yes, it's not properly documented yet, but all atomsk flags are supported. For dislocations it would be something like

pr.create.structure.atomsk.create('fcc', 3.6, 'Cu').duplicate(5, 1, 5).dislocation('0.498*box',  '0.500*box', 'edge_add', 'Y', 'Z', 3.6, 0.217).build()

where the last two numbers are the burgers vector and poisson ratio.

pmrv · 2021-06-29T08:28:51Z

Nice! Is there a way to include dislocations? doesn't have to be this PR.

Yes, it's not properly documented yet, but all atomsk flags are supported. For dislocations it would be something like
pr.create.structure.atomsk.create('fcc', 3.6, 'Cu').duplicate(5, 1, 5).dislocation('0.498*box',  '0.500*box', 'edge_add', 'Y', 'Z', 3.6, 0.217).build()
where the last two numbers are the burgers vector and poisson ratio.

The pattern is that all method calls on AtomskBuilder are translated like
method_call(a1, a2, ...) -> -method-call a1 a2 ...

liamhuber

Very nice, I'm happy (a) this happened so quickly, and (b) that you found an easy and direct way to bind things. A few discussion points follow.

pyiron_atomistics/atomistics/structure/factories/atomsk.py

liamhuber · 2021-06-29T09:33:05Z

pyiron_atomistics/atomistics/structure/factories/atomsk.py

+        with tempfile.TemporaryDirectory() as temp_dir:
+            if self._structure is not None:
+                write(os.path.join(temp_dir, "input.exyz"), self._structure, format="extxyz")
+            proc = subprocess.run(["atomsk", *" ".join(self._options).split()],


I think we need to do something about the dependencies. Is it sufficient to conda-install atomsk to get this running, or would I further need to modify the command line name after installation? Is atomsk available across all architectures? How can we ImportAlarm something that's not a python package? Since we're working in atomistics and not contrib I'd like to get at least some of these answered before we merge...

Installing the conda package is enough, but there's only a linux build on conda, not sure if a windows build is possible. Using ImportAlarm is not possible since it's not a python package, but I could modify the StructureFactory to only export atomsk if it is found in the PATH.

liamhuber · 2021-06-29T09:34:35Z

pyiron_atomistics/atomistics/structure/factories/atomsk.py

+    pbc: [ True  True  True]
+    cell: 
+    Cell([10.2, 3.6, 3.6])
+    """


copypasta examples to unit tests folder? Will need to be excluded from the CI until we get the dependencies better sorted, but should at least be there for local machines to run.

pmrv · 2021-06-30T18:28:32Z

Tests and docstrings are in, so this is done from my side.

@zhuochengXIE @LVC14C15 Can you have a look if this meets your needs (or is understandable at all)?

Co-authored-by: Liam Huber <liam.huber@gmail.com>

stale · 2021-07-15T00:54:41Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jan-janssen · 2021-07-22T17:36:08Z

Nice! Is there a way to include dislocations? doesn't have to be this PR.

Did not you do this in https://github.com/eisenforschung/pyiron_mpie/blob/master/pyiron_mpie/interactive/dislocation.py already? I thought we already had an atomsk wrapper, did not we?

pmrv · 2021-07-27T10:15:29Z

I found one place in pyiron_mpie that uses atomsk before here. The reason to add the wrapper here was not just dislocations though, but that Eric's group is using it generally a lot.

stale · 2021-08-10T14:43:26Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

pmrv · 2021-08-11T11:35:41Z

Previously I used a feature introduced here that is not yet available in the conda version. I have therefore rewritting the part calling atomsk to avoid it.

This is now done from my side, unless there's additional feature requests.

jan-janssen · 2021-08-11T14:16:27Z

Previously I used a feature introduced here that is not yet available in the conda version. I have therefore rewritting the part calling atomsk to avoid it.

To be future proof I would suggest to update the conda package - I opened a corresponding pull request here conda-forge/atomsk-feedstock#14 - the issues is that they no longer publish tagged releases on Github https://github.com/pierrehirel/atomsk/tags so in case anybody is in contact with the developer that is maybe something we can ask them for.

jan-janssen · 2021-08-11T14:48:45Z

I updated the conda package 0.11 and asked the developer to update their tags on Github pierrehirel/atomsk#35

liamhuber · 2021-08-20T10:44:12Z

tests/atomistics/structure/factories/test_atomsk.py

+from pyiron_atomistics._tests import TestWithProject
+from pyiron_atomistics.atomistics.structure.factories.atomsk import AtomskFactory, AtomskError
+
+if shutil.which("atomsk") is not None:


I'm a little uncomfortable hard-coding the executable name (e.g. what if someone has multiple atomsk versions installed and renames them accordingly?) but I don't think it needs to stop the merge.

Agree, ideally this would somehow reuse the Executable class that we have already for jobs, so you'd be able to configure your particular atomsk, but I'm drawing a blank how to actually do it right now.

stale · 2021-09-03T14:00:34Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

pmrv · 2021-09-06T13:23:05Z

Latest version is up on conda now.

This reverts commit 3a6a247.

This will ensure it's only available when installed

pmrv · 2021-09-08T10:28:49Z

I've made it so now, that the code is only exported in the StructureFactory when the atomsk executable is available.

pmrv added 2 commits June 29, 2021 09:50

Add atomsk wrapper

80b228c

Add simple docstring

22440e5

pmrv added the enhancement New feature or request label Jun 29, 2021

pmrv requested a review from liamhuber June 29, 2021 08:04

liamhuber reviewed Jun 29, 2021

View reviewed changes

pmrv added 5 commits June 29, 2021 16:58

Add docstring to create

c447dd2

Document method translation scheme

688bf83

Document builder methods

884a4cc

Add tests

1421bee

Add tests and raise error on atomsk failure

0bb1b98

pmrv and others added 3 commits June 30, 2021 20:32

Add metadata

2162085

Simplify hkl check

07eeddb

Co-authored-by: Liam Huber <liam.huber@gmail.com>

Add missing import

bdea27c

stale bot added the stale label Jul 15, 2021

pmrv removed the stale label Jul 16, 2021

pmrv mentioned this pull request Jul 28, 2021

Feature request: Add option to redirect output to stdout rather than writing to disk pierrehirel/atomsk#30

Closed

stale bot added stale and removed stale labels Aug 10, 2021

Use a named output file instead of stdout

3a6a247

Remove missing import

9c31b62

liamhuber approved these changes Aug 20, 2021

View reviewed changes

stale bot added the stale label Sep 3, 2021

pmrv removed the stale label Sep 3, 2021

pmrv added 2 commits September 8, 2021 12:22

Revert "Use a named output file instead of stdout"

1d0d9dd

This reverts commit 3a6a247.

Guard atomsk code behind shutil.which("atomsk")

99591b7

This will ensure it's only available when installed

pmrv requested a review from liamhuber September 8, 2021 10:28

Correct import and fix check for binary

9f3a16e

pmrv merged commit 3b7f9b6 into master Sep 16, 2021

delete-merged-branch bot deleted the atomsk branch September 16, 2021 09:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Wrapper for Atomsk Structure Creation #260

Add Wrapper for Atomsk Structure Creation #260

pmrv commented Jun 29, 2021

samwaseda commented Jun 29, 2021

coveralls commented Jun 29, 2021 •

edited

Loading

pmrv commented Jun 29, 2021 •

edited

Loading

pmrv commented Jun 29, 2021

liamhuber left a comment

liamhuber Jun 29, 2021

pmrv Jun 29, 2021

liamhuber Jun 29, 2021

pmrv commented Jun 30, 2021

stale bot commented Jul 15, 2021

jan-janssen commented Jul 22, 2021

pmrv commented Jul 27, 2021

stale bot commented Aug 10, 2021

pmrv commented Aug 11, 2021 •

edited

Loading

jan-janssen commented Aug 11, 2021

jan-janssen commented Aug 11, 2021

liamhuber Aug 20, 2021

pmrv Aug 20, 2021

liamhuber Aug 20, 2021

stale bot commented Sep 3, 2021

pmrv commented Sep 6, 2021

pmrv commented Sep 8, 2021

Add Wrapper for Atomsk Structure Creation #260

Add Wrapper for Atomsk Structure Creation #260

Conversation

pmrv commented Jun 29, 2021

samwaseda commented Jun 29, 2021

coveralls commented Jun 29, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1237696022

💛 - Coveralls

pmrv commented Jun 29, 2021 • edited Loading

pmrv commented Jun 29, 2021

liamhuber left a comment

Choose a reason for hiding this comment

liamhuber Jun 29, 2021

Choose a reason for hiding this comment

pmrv Jun 29, 2021

Choose a reason for hiding this comment

liamhuber Jun 29, 2021

Choose a reason for hiding this comment

pmrv commented Jun 30, 2021

stale bot commented Jul 15, 2021

jan-janssen commented Jul 22, 2021

pmrv commented Jul 27, 2021

stale bot commented Aug 10, 2021

pmrv commented Aug 11, 2021 • edited Loading

jan-janssen commented Aug 11, 2021

jan-janssen commented Aug 11, 2021

liamhuber Aug 20, 2021

Choose a reason for hiding this comment

pmrv Aug 20, 2021

Choose a reason for hiding this comment

liamhuber Aug 20, 2021

Choose a reason for hiding this comment

stale bot commented Sep 3, 2021

pmrv commented Sep 6, 2021

pmrv commented Sep 8, 2021

coveralls commented Jun 29, 2021 •

edited

Loading

pmrv commented Jun 29, 2021 •

edited

Loading

pmrv commented Aug 11, 2021 •

edited

Loading