CP2K enhancements

[patrickmelix]

Hey,

just saw that there are some minor changes in #66 that affect the CP2K interface I'm also still working on. Sorry for not putting that here earlier, didn't know somebody else is working on it too and was busy with something else in the meantime.

I also added the unit option for the energies here, so this might conflict with #66. Also there's now some things around MDs.

Not final yet, feel free to request changes (especially @BvB93 ).
Cheers,
Patrick

[BvB93]

Hi Patrick,

The changes you've made look just fine to me.
As you've already added the unit keyword in your branch, i'll remove it from #66 for the sake of minimizing redundancy.

Regards, Bas

[patrickmelix]

Awesome, thanks. And sorry for not pushing earlier...

[patrickmelix]

I think nothing more is coming any time soon from me. Sorry for the delay. Could be merged.

[patrickmelix]

Cleaning my ToDo-List: Anything more for me to do @BvB93?

[BvB93]

A few, mostly style-related, comments but overall I feel these are nice additions.
The only thing that I find a bit concerning is Cp2kResults.get_energy() returning a list rather than a float, as it is the only Job.get_energy() which does so.

Secondly, Michal just said that he'll take a look at the open PRs, which should help with the whole merging thing.

[patrickmelix]

Thx for the input @BvB93, very good!

[BvB93]

Really liking the changes!
I've got a few more documentation-related questions/remarks but in my opinion the PR is good to go afterwards.

[BvB93]

By the way, I assume these arrays are of all different shapes and thus we can't coerce the final result into a single array?

[patrickmelix]

I guess in almost all cases they will be the same length. But in any simulation without constant number of particles, the array length might differ between frames. Not sure if there is any calculation in CP2K that allows this (perhaps some embedded schemes?!), but I guess this way we are on the safe side...

[BvB93]

I guess in almost all cases they will be the same length.

By the sound of it the this would entail the vast majority (all?) cases.

How about converting the whole thing into an ndarray via a try / except approach,
using a ragged array as fallback if necessary?

import warnings
import numpy as np

def _sequence_to_array(seq, dtype=float):
    """Convert the (nested) sequence *seq* into an array with the specified *dtype*.
    
    If *seq* is ragged sequence it will be converted into a ragged object array instead.
    """
    try:
        return np.array(seq, dtype=dtype)
    except ValueError as ex:
        # Create and return a ragged array, 
        # i.e. an object array whose elements are normal arrays
        ret = np.empty(len(seq), dtype=object)
        ret[:] = [np.array(item, dtype=dtype) for item in seq]

        warning = RuntimeWarning(f"Failed to coerce the forces into a single {np.dtype(dtype)} array;"
                                 "returning a ragged array instead")
        warning.__cause__ = ex
        warnings.warn(warning, stacklevel=3)
        return ret

class Cp2kResults:
    get_forces(...):
        ...
        return _sequence_to_array(ret)

[BvB93]

@felipeZ do you have any comments?
Considering this entails a Cp2kJob / Cp2kResult enhancement.