search

aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
436 stars 190 forks source link

get_class_to_entry_point_map of pluginloader is broken #1066

Closed sphuber closed 6 years ago

sphuber commented 6 years ago

In the get_entry_map call this function hard codes the aiida-core as argument, which will limit the entries to those of the aiida-core distribution. This means that any entries installed by other distributions will be ignored, which is clearly problematic.

sphuber commented 6 years ago

Even though the problem can be solved by removing the aiida-core argument from the get_entry_map call (and upgrading reentry to version 1.0.3 where the distribution name became optional, I think there still is a bigger problem with the type strings.

The reason I stumbled upon this problem is because currently the WorkCalculation nodes that get stored when a WorkChain is ran, do not keep any information as to which class it was that ran it. Afterwards it is therefore impossible to determine what the WorkChain class was. We thought of storing a sort of type string in the attributes of the WorkCalculation. My initial idea was to stick close to the plugin system, which seems to be working well, and invert the class name to the plugin category and entry point name. For example if we have an instance of PwBaseWorkChain calling a function like entry_point_from_class would give us a tuple ('aiida.workflows', 'quantumespresso.pw.base'). The two strings in this tuple are "native" to the plugin loading system and the WorkflowFactory could easily be leveraged to recreate the correct class.

The downside of this approach is that it is a different way than the JobCalculations currently implement or define the type string, which could make it confusing, more so than it already is. For example using the already existing entry_point_tpstr_from function there already are inconsistencies

In [6]: from aiida.common.pluginloader import entry_point_tpstr_from, get_entry_point_from_class

In [7]: entry_point_tpstr_from(PwCalculation)
Out[7]: u'calculation.job.quantumespresso.pw.PwCalculation.'

In [8]: entry_point_tpstr_from(PwBaseWorkChain)
Out[8]: u'aiida.workflows.quantumespresso.pw.base.PwBaseWorkChain.'

@giovannipizzi @DropD @muhrin Maybe this is the moment to really sit down and decide how we want these type string to work and if necessary redefine them in a consistent way across all the old and new components and if need be write a migration script.

muhrin commented 6 years ago

Fixed by PR #1294