Rename node class for JobCalculation#2201
Conversation
The node class changed from `JobCalculation` to `CalcJobNode`. Since the `JobCalculation` class is heavily used by users, as it is the class that is sub classed to define a job calculation, we cannot quite get rid of it. Instead we moved all the logic from the old `Calculation` and `JobCalculation` classes to their new equivalents `ProcessNode` and `CalcJobNode`, and put an alias to `CalcJobNode` for `JobCalculation`. This last class can now still be imported from `aiida.orm.calculation` and `aiida.orm.calculation.job`. Note that the paths that import directly from the `implementation.general` sub module have been removed, because the job calculation classes are now fully concrete. Furthermore, the old entry points for the base calculation classes `inline`, `job` `work` and `function` have been removed as they have been replaced with the entry points in the `aiida.node` group for the new node classes.
|
|
||
| # This is an example of a query that could be overriden by a better implementation, | ||
| # for performance reasons: | ||
| def query_jobcalculations_by_computer_user_state( |
There was a problem hiding this comment.
Are these just renamed or completely removed? These were here because they could be made faster by custom implementations of the query (how does now verdi calculation list get the list of non-finished calculations, and is this fast?
There was a problem hiding this comment.
I removed them because they were not being used, and they are outdated. verdi calculation list uses the JobCalculation._list_calculations classmethod, which is also not ideal, but that has been the situation for a long time. Currently, it is the process state that determines "activeness" of a process and hence calculation, the old calc_state is merely an additional attribute.
| module, class_name = calculation_class.rsplit('.', 1) | ||
|
|
||
| # For the base class 'calculation.job.JobCalculation' the module at this point equals 'calculation.job' | ||
| # For the base class 'calculation.job.CalcJobNode' the module at this point equals 'calculation.job' |
There was a problem hiding this comment.
This comment is outdated? (calculation.job.CalcJobNode -> node.process.calculation.calcjob.CalcJobNode?)
| # Find a new subfolder. | ||
| # I do not user tempfile.mkdtemp, because it puts random characters | ||
| ## Find a new subfolder. | ||
| ## I do not user tempfile.mkdtemp, because it puts random characters |
There was a problem hiding this comment.
Why two # characters? Either also for the lines below or don't add the second #
There was a problem hiding this comment.
Mistake. In first commit of this branch, I copied over content of JobCalculation and already cleaned it a bit such as these double comment characters. But then in following commits I changed JobCalculation a bit and to make sure that I included everything in this PR, I simply pasted the JobCalculation code once more in CalcJobNode, which undid my removal of the two characters. Long story short, I will remove them again :)
| 'simpleplugins.arithmetic.add = aiida.orm.calculation.job.simpleplugins.arithmetic.add:ArithmeticAddCalculation', | ||
| 'simpleplugins.templatereplacer = aiida.orm.calculation.job.simpleplugins.templatereplacer:TemplatereplacerCalculation', | ||
| 'arithmetic.add = aiida.calculations.plugins.arithmetic.add:ArithmeticAddCalculation', | ||
| 'templatereplacer = aiida.calculations.plugins.templatereplacer:TemplatereplacerCalculation', |
There was a problem hiding this comment.
Should we add entries for arithmetic and templatereplacer in the plugin registry, to reserve the names?
There was a problem hiding this comment.
Yes, if you agree with the renaming, we should do this. I did not like the simpleplugins prefix, but if we could find a good namespace to put these internal ones in that'd be ok too. If we think it is not a problem to reserve these namespace, then we should register them on the registry
| # For the base class 'calculation.job.CalcJobNode' the module at this point equals 'calculation.job' | ||
| # For this case we should simply set the type to the base module calculation.job. Otherwise we need | ||
| # to strip the prefix to get the proper sub module | ||
| # For the base class 'mode.process.calculation.calcjob.CalcJobNode' the module at this point equals |
There was a problem hiding this comment.
There's a typo here but we can fix in the next PR
4 participants
Fixes #2199
The node class changed from
JobCalculationtoCalcJobNode.Since the
JobCalculationclass is heavily used by users, as it is theclass that is sub classed to define a job calculation, we cannot quite get
rid of it. Instead we moved all the logic from the old
CalculationandJobCalculationclasses to their new equivalentsProcessNodeandCalcJobNode,and put an alias to
CalcJobNodeforJobCalculation.This last class can now still be imported from
aiida.orm.calculationandaiida.orm.calculation.job. Note that the paths that import directly from theimplementation.generalsub module have been removed, because the job calculationclasses are now fully concrete.
Furthermore, the old entry points for the base calculation classes
inline,jobworkandfunctionhave been removed as they have been replaced with the entrypoints in the
aiida.nodegroup for the new node classes.