lsmo-epfl / aiida-lsmo

AiiDA workflows for the LSMO laboratory at EPFL
Other
9 stars 13 forks source link

Aiida 1.0 develop #9

Closed danieleongari closed 4 years ago

danieleongari commented 4 years ago

Please merge this PR and make the first alpha release.

yakutovicha commented 4 years ago

Something weird is happening with the python function decorated as calcfunction

If I try to load it using the CalculationFactory, I get the following error:

In [1]: FFBuilder = CalculationFactory('lsmo.ff_builder')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-866042a89048> in <module>()
----> 1 FFBuilder = CalculationFactory('lsmo.ff_builder')

/Users/yakutovicha/Documents/aiida-core/aiida/plugins/factories.py in CalculationFactory(entry_point_name)
     63     valid_classes = (CalcJob, calcfunction)
     64
---> 65     if issubclass(entry_point, CalcJob):
     66         return entry_point
     67

/Users/yakutovicha/anaconda3/envs/venv-aiida/lib/python3.7/abc.py in __subclasscheck__(cls, subclass)
    141         def __subclasscheck__(cls, subclass):
    142             """Override for issubclass(subclass, cls)."""
--> 143             return _abc_subclasscheck(cls, subclass)
    144
    145         def _dump_registry(cls, file=None):

TypeError: issubclass() arg 1 must be a class

The same happens on travis.

yakutovicha commented 4 years ago

The same happens if I use your branch (not squashed and not rebased on top of aiida-1.0)

ltalirz commented 4 years ago

I don't think this is very weird - the entry point points to the module, not to the calcfunction Should be ff_builder.ff_builder

ltalirz commented 4 years ago

just pinging @sphuber to mention that the check introduced on the factories has surfaced the first bug

Perhaps we could work on the error message - here, a module is passed instead of a class and issubclass() fails. In general, entry points can point to any python object. Would be nicer to try/except and to print a message saying: got "module", expecting a calculation class.

Do you want to look into this or should I? Just let me know

yakutovicha commented 4 years ago

I don't think this is very weird - the entry point points to the module, not to the calcfunction Should be ff_builder.ff_builder

The weird thing is it works fine on Daniele's computer

ltalirz commented 4 years ago

Maybe older AiiDA version? These checks were introduced relatively recently (but before 1.0 I believe)

yakutovicha commented 4 years ago

I don't think this is very weird - the entry point points to the module, not to the calcfunction Should be ff_builder.ff_builder

Also, looking at this line https://github.com/yakutovicha/aiida-lsmo/blob/77d62695cf81f28144c3c9d8137784a4c959dd42/aiida_lsmo/calcfunctions/__init__.py#L2

I think the same name for module and function might confused python.

yakutovicha commented 4 years ago

Maybe older AiiDA version? These checks were introduced relatively recently (but before 1.0 I believe)

He has 1.0

yakutovicha commented 4 years ago

Do you want to look into this or should I? Just let me know

I take care of the PR to aiida-core

ltalirz commented 4 years ago

I think the same name for module and function might confused python.

I see, you're right. I would not know by heart which takes precedence in this case. It would indeed be a bit worrying if this behavior varies between python versions/implementations. Perhaps it is even influenced by some .pyc files lying around, or an outdated entry point map?

sphuber commented 4 years ago

There are two problems, one here and one in aiida-core. The problem here is the entry point:

"lsmo.ff_builder = aiida_lsmo.calcfunctions:ff_builder"

should be

"lsmo.ff_builder = aiida_lsmo.calcfunctions.ff_builder:ff_builder"

I see that you tried to "forward" it by importing it on the calcfunctions module level, which maybe should be possible for entry points, but in this case I think it is better to be explicit.

The second problem is in aiida-core where it indeed assumes that whatever the entry point is pointing to can be used in issubclass which clearly is not the case. This should be fixed

yakutovicha commented 4 years ago

We discovered a funny difference between python 3.6 and 3.7:

for python 3.6 the following code works:

In [4]: def test():
    ...:     pass
    ...:

In [5]: issubclass(test, CalcJob)
Out[5]: True

for python 3.7 it doesn't:

In [10]: def test():
    ...:     pass
    ...:

In [11]: issubclass(test, CalcJob)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-11-07b7d3b7b23d> in <module>()
----> 1 issubclass(test, CalcJob)

/Users/yakutovicha/anaconda3/envs/venv-aiida/lib/python3.7/abc.py in __subclasscheck__(cls, subclass)
    141         def __subclasscheck__(cls, subclass):
    142             """Override for issubclass(subclass, cls)."""
--> 143             return _abc_subclasscheck(cls, subclass)
    144
    145         def _dump_registry(cls, file=None):

TypeError: issubclass() arg 1 must be a class
ltalirz commented 4 years ago

Interesting!