Sx run addon

[freyso]

new utility function to call postprocessing addon for SPHInX job

[jan-janssen]

This is rather cluster specific, is it possible to move this to the pyiron resources?

[jan-janssen]

Here is an example how we do it for sxextopt https://github.com/pyiron/pyiron_atomistics/blob/master/pyiron_atomistics/interactive/sxextoptint.py#L64

[freyso]

agreed in principle.
However, I refuse to use the job.Executable mechanism because there are dozens of possible add-ons, and the module versions are very dynamic. I am not going to create hundreds of wrapper scripts for that purpose. The entire run_addon idea is to leverage that complexity.

Rough idea:

Approach will be to have a get_sx_versions script that generates the info dynamically; this will be located in the sphinx/ subfolder of the ressource_path and hence know about the specific requirements of the cluster.
How do I put a "default" get_sx_versions for the standard pyiron installation? I.e., where does the anaconda share/pyiron/sphinx/ folder come from?
The one for the cmmc will be created separately, and I know how to do that.

The info I need will be pairs of - this is essentially dictionary-style data.
Is there a prefered format for config files for such data? I can do
=
or
json
{ version: "", pathcommand: "" }
or anything else.

For the cluster, this will be simply
2.6.3=module load sphinx/2.6.3
3.0.8-openmp=module load sphinx/3.0.8-openmp
etc.

For addons in the standard path, this could be
default=

For addons with a specific path, this could be
devel=export PATH=$HOME/devel/sphinx-debug/bin/:${PATH}

so there's a whole lot of flexibility of how to enable different code versions.

[jan-janssen]

@freyso An alternative idea would be to provide these utilities as part of the sphinxdft conda package, then we do not need a separate module load and other people who just test S/PHI/nX also have access to them. Are they included in our current package? Is it difficult to include them? Are they performance critical?

[freyso]

I critically need the versioning for the addons.
Some add-ons may be performance-sensitive. Some add-ons change over time (added or removed features). And the wavefunctions can have hdf5 format or netcdf format, and this is compiled into the code (which is one of the main motivations for this entire branch).
The current solution is 100% transparent, all versioning is provided by the local installation, no hardcoded/defaulted versions from pyiron.

[freyso]

but yes, all SPHInX addons should be part of the sphinxdft conda package. So in principle, they are available to testers, too (provided that the search paths are properly set), and many of them are working across different SPHInX versions.

[jan-janssen]

I critically need the versioning for the addons.

I agree - I am just afraid that it becomes a sphinx specific solution.

[freyso]

Well, for the time being, it is a SPHInX-specific problem...
It's probably related to the fact that I am currently the only one who is trying to do active code development while using pyiron. So my need for versioning is much higher than for most others, and my requirements for having full access to even the most bizarre features of the code as well.
Please give me some freedom in trying out possible solutions in this corner.

[freyso]

@jan-janssen: I don't understand the unit test error. Local tests in mybinder produce a two-species potential list for sphinx_test_2_5 as expected from the input (Fe, Ni). The test run by github seems to always list Fe only (which I believe to be incorrect).
I had believed it is some kind of race condition with a competing test (same job name, different structure) that is run in parallel, as this was the origin of the test error for my new test case, but I cannot find such a competing test. I also speculated about old data sets from a previous test version, but I cannot verify.
Do you have any idea?
Otherwise, what should we do? Comment out the problematic, non-reproducible assert?

[niklassiemer]

On github we test using the resources from the 'tests/static' directory, i.e. https://github.com/pyiron/pyiron_atomistics/tree/master/tests/static/sphinx for sphinx and inside there, only one species (Fe) is defined. (For testing, we do not install all the optional/code dependencies but only the minimum as defined in https://github.com/pyiron/pyiron_atomistics/blob/master/.ci_support/environment.yml). This ensures stable resources for our testing, such that we can e.g. rest list_potentials with a fixed known list of available potentials and that does not break on an update of a potentials provider package.

I actually would be in favor of separating static files we do tests on and the 'test resources' - and add documentation about our test environment!

[freyso]

Ok, but then this particular assert is meaningless as a unit test, because it tests a (somewhat random) environment rather than the source code. If there is no easy way to enforce a stable environment, I strongly suggest to comment out this very check.
Even if we decided to add the Ni_GGA potential to the static environment, there still could be installations with additional Fe,Ni potentials, and the test would falsely fail again.
@jan-janssen, @niklassiemer : ok for you that I disable the potential list check for the Fe+Ni structure for the time being?
Alternatively, one could fix the static ressources and simply check that the Fe_GGA and Ni_GGA potentials are contained in the list (which may or may not contain additional ones). This change could then (later) be extended to the Fe-only tests, if these ever fail.
Opinions?

[niklassiemer]

Well, it tests if list_potentials (our sourcecode) provides exactly the expected output for a given environment (being the input in this case). In an unknown environment we could not test that at all!
Ok, probably the different versions of sphinx read that in the same way (which I do not know).

Of course we should document how to run our tests, which at various places rely on the static folder being the resources. I would like the following:

A folder in the tests named test_recources that contain the resource files only.
A specific unittest suite that is used if one calls discover on the tests folder (i.e. set the resources before calling discover on the subdirectories). That way the unittest run the same for everyone and independent from the normal settings of the user!

[coveralls]

Pull Request Test Coverage Report for Build 3622695040

• 114 of 124 (91.94%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 68.822%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pyiron_atomistics/sphinx/base.py	67	72	93.06%
pyiron_atomistics/sphinx/util.py	47	52	90.38%

Totals
Change from base Build 3622625190:	0.2%
Covered Lines:	12227
Relevant Lines:	17766

---

💛 - Coveralls

[niklassiemer]

Actually, that the addon itself does not run on other OS than Linux is fine for me. However, which version of a post-processing script is available should be also accessible from windows or mac. This would be in line with our other resources where we allow to build up a VASP job under windows and sent it to a remote machine (running on Linux) to be executed. Therefore, I would like this to be running OS independent. Can the script be converted to a python one?

[freyso]

This is no problem at all. If anything else fails, I can work with wrappers to keep my favorite language on the local install.

But I have little experience with python under other OSs. What would the best way to run a python script in a portable way and capture the (json) output? Is for instance subprocess("script.py", shell=True) portable?

[niklassiemer]

Hmm, looking at the ScriptJob which runs python code we use f'python {absolute_path_to_script}' in the subprocess call:
https://github.com/pyiron/pyiron_base/blob/7ffb98d87f610967cd3c141d787bba7f83c687db/pyiron_base/jobs/script.py#L372-L400

And that is used in all OSs.

[freyso]

@niklas, @jan-janssen
So, the script-based sxversions detection is now based on python script rather than shell script, and hence the test case runs under Windows, too.
The actual add-ons do not run under Windows, so I cannot test. For the Linux test, I faked an executable with shell. On windows, I don't have a shebang mechanism to run a fake executable directly.
The code quality check complains about my using subprocess.run (shell=True) for security reasons, but I do not see a sensible workaround for this.
I hope this satisfies your requirements now. I have spent a lot more time on this than anticipated; this was meant to merely be the preparation for something interesting.

[stale]

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.

[freyso]

@jan-janssen @niklassiemer : What is the merge policy here? Do I merge myself? Do you merge? Or still a problem here?

[stale]

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.