aiida-core icon indicating copy to clipboard operation
aiida-core copied to clipboard

Calcfunctions do not properly handle `*args`

Open yakutovicha opened this issue 11 months ago • 2 comments

Describe the bug

This is a proper behaviour of Python function:

from aiida import orm

In [26]: x = [orm.Int(1), orm.Int(2)]

In [27]: def f(*x, y=True):
    ...:     print(x)
    ...:     print(y)

In [29]: f(*x, y=False)
(<Int: uuid: 83fcb928-cc6b-4279-aa1b-f37aede53518 (unstored) value: 1>, <Int: uuid: 223b032d-d0f2-42f3-97dd-c328b59df66c (unstored) value: 2>)
False

This is the (assumingly wrong) behaviour of the corresponding calcfunction:

In [28]: @engine.calcfunction
    ...: def g(*x, y=True):
    ...:     print(x)
    ...:     print(y)

In [30]: g(*x, y=False)
(<Int: uuid: 83fcb928-cc6b-4279-aa1b-f37aede53518 (pk: 2366) value: 1>, <Int: uuid: 223b032d-d0f2-42f3-97dd-c328b59df66c (pk: 2367) value: 2>)
True  # < ----- Should be False

Expected behaviour

I would expect that calcfunctions behave exactly as standard Python functions

Additional context

My goal is to implement a trajectory merge function that accepts frames with the same IDs or keeps only the unique ones. The desired signature of the function would look like this:

@engine.calcfunction
def merge_trajectory_data(*trajectories, unique_stepids=False):

Due to the current issue, I will implement two methods: merge_trajectory_data_non_unique and merge_trajectory_data_unique. Unless there are better ways to do it, of course.

yakutovicha avatar Mar 07 '24 09:03 yakutovicha

I've checked the PR that implemented this feature in #5691, and indeed there are no testcases covering the interaction of variadic and keyword arguments. https://github.com/aiidateam/aiida-core/pull/5691/files

CC @sphuber

danielhollas avatar Mar 13 '24 15:03 danielhollas

What version of aiida-core are you using @yakutovicha ? I cannot reproduce the problem on the latest release:

In [1]: from aiida import engine, orm

In [2]: x = [orm.Int(1), orm.Int(2)]

In [3]: @engine.calcfunction
   ...: def g(*x, y=True):
   ...:     print(x)
   ...:     print(y)
   ...: 

In [4]: g(*x, y=False)
(<Int: uuid: 1e028e36-8b23-4e58-9e96-47e23905b9b7 (pk: 157603) value: 1>, <Int: uuid: 543757aa-f00f-494a-8d23-298d026009e8 (pk: 157604) value: 2>)
uuid: 59a45829-18e2-44ec-8244-b7bdf8bb9bfd (pk: 157602) value: False
Out[4]: {}

I think this may have been fixed in this commit: https://github.com/aiidateam/aiida-core/commit/ca8bbc67fcb40d6cec4e1cae32ce114495c0eb1d which was released with v2.5

sphuber avatar Mar 13 '24 17:03 sphuber

I'm sorry for not getting back to you sooner - I used AiiDA v2.4.3. The issue is solved in the newest releases 👍

yakutovicha avatar Apr 23 '24 11:04 yakutovicha