nomad-coe / atomistic-parsers

Apache License 2.0
7 stars 3 forks source link

Errors in Gromacs parsing after NOMAD rebase #20

Closed JFRudzinski closed 1 year ago

JFRudzinski commented 1 year ago

Problem: Several gromacs tests fail after rebasing NOMAD.

More specifically, line 1094 of parser.py in gromacs/ throws an error having to do with converting a str input parameter like '0 0 0' when running test_md_verbose (equivalently, you can simply try to parse the fe_test data under the gromacs test data folder, see error details below).

nb - This is specific to the gromacs parser, lammps tests work fine.

Relevant NOMAD branch: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/tree/1157-add-rg-plots-to-overview-gui-page-for-molecular-dynamics-trajectories

Relevant atomistic parser branch: develop

Error details:

============================================================================== FAILURES ==============================================================================
__________________________________________________________________________ test_md_verbose ___________________________________________________________________________

parser = <atomisticparsers.gromacs.parser.GromacsParser object at 0x7f2390d6a710>

    def test_md_verbose(parser):
        archive = EntryArchive()
>       parser.parse('tests/data/gromacs/fe_test/md.log', archive, None)

tests/test_gromacsparser.py:37: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
atomisticparsers/gromacs/parser.py:1162: in parse
    self.parse_input()
atomisticparsers/gromacs/parser.py:1094: in parse_input
    setattr(sec_control_parameters, key, val)
../../../nomad/metainfo/metainfo.py:1236: in __setattr__
    return super().__setattr__(name, value)
../../../nomad/metainfo/metainfo.py:3209: in __set__
    obj.m_set(self, value)
../../../nomad/metainfo/metainfo.py:1342: in m_set
    value = to_numpy(quantity_def.type, quantity_def.shape, quantity_def.unit, quantity_def, value)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

np_type = <class 'numpy.float64'>, shape = [3], unit = None
definition = atomisticparsers.gromacs.metainfo.gromacs.x_gromacs_section_control_parameters.x_gromacs_inout_control_acc:Quantity, value = '0           0           0'

    def to_numpy(np_type, shape: list, unit: Optional[pint.Unit], definition, value: Any):
        check_dimensionality(definition, unit)

        if isinstance(value, pint.Quantity):
            # if flexible unit is set, do not check unit in the definition
            # it will be handled specially
            # the stored unit would not be serialized
            flexible_unit = getattr(definition, 'flexible_unit', False)

            if not flexible_unit and unit is None:
                raise TypeError(f'The quantity {definition} does not have a unit, but value {value} does.')

            if type(value.magnitude) == np.ndarray and np_type != value.dtype:
                value = value.astype(np_type)

            if not flexible_unit:
                value = value.to(unit).magnitude
            else:
                value = value.magnitude

        if isinstance(value, pd.DataFrame):
            try:
                value = value.to_numpy()
            except AttributeError:
                raise AttributeError(
                    f'Could not convert value {value} of type pandas.Dataframe to a numpy array')

        if np_type in MTypes.complex:
            value = normalize_complex(value, np_type, unit)

        if type(value) != np.ndarray:
            if len(shape) > 0:
                try:
>                   value = np.asarray(value, dtype=np_type)
E                   ValueError: could not convert string to float: '0           0           0'

../../../nomad/metainfo/util.py:730: ValueError

@ladinesa : Could you take a look at this? I don't want to make changes to fix this single problem, because I don't really understand more generally all the possible cases.

ladinesa commented 1 year ago

It is kind of strange because the type for all of these x_gromacs_inout_control quantities is str and shape == 0. So it should not try to convert. Can you please try to remove the shape attribute in the metainfo defs? Otherwise, please create an issue in gitlab so Theodore can address it. It is coming from the metainfo.

JFRudzinski commented 1 year ago

oh ok, I see now. It is trying to convert because the metainfo says it should be a float of shape=[3]:

    x_gromacs_inout_control_acc = Quantity(
        type=np.dtype(np.float64),
        shape=[3],
        description='''
        Gromacs running environment and control parameters.
        ''',)

But I am not sure why either 1. The parameter is now initially being read as a string or 2. The conversion used to work but now does not (I guess this is more likely)?

Does this clarify anything for you in terms of what has changed? Or should I still talk to Theodore?

ladinesa commented 1 year ago

Ah I miss this, can you simply set it to string. Not sure why it was working before. It this solves the problem, there is no need to talk to him.

JFRudzinski commented 1 year ago

Ok, great. It works now. I will created a branch for this issue and then make the quick change before making a pull request.