aiidateam / aiida-atomistic

AiiDA plugin which contains data and methods for atomistic simulations.
https://aiidateam.github.io/aiida-atomistic/
MIT License
3 stars 7 forks source link

`to_ase` error when kind name includes string tag. #26

Open superstar54 opened 1 month ago

superstar54 commented 1 month ago
structure_dict = {
    'cell':[[2.75,2.75,0],[0,2.75,2.75],[2.75,0,2.75]],
    'pbc': [True,True,True],
    'sites':[
        {
            'symbol':'Si',
            'position':[3/4, 3/4, 3/4],
            'charge': +1,
            'kind_name': 'Si_up',
        },
        {
            'symbol':'Si',
            'position':[1/2, 1/2, 1/2],
            'kind_name': 'Si_down',
        },
    ],
}

mutable_structure = StructureDataMutable(**structure_dict)
structure = StructureData(**structure_dict)
structure.to_ase().get_tags()

Error message

structure_dict = {
    'cell':[[2.75,2.75,0],[0,2.75,2.75],[2.75,0,2.75]],
    'pbc': [True,True,True],
    'sites':[
        {
            'symbol':'Si',
            'position':[3/4, 3/4, 3/4],
            'charge': +1,
            'kind_name': 'Si_up',
        },
        {
            'symbol':'Si',
            'position':[1/2, 1/2, 1/2],
            'kind_name': 'Si_down',
        },
    ],
}

mutable_structure = StructureDataMutable(**structure_dict)
structure = StructureData(**structure_dict)
structure.to_ase().get_tags()
mikibonacci commented 1 month ago

Hi @Xing, thanks a lot for testing the StructureData. In this case, the issue is in the ASE object: the tags are strings ('_up','_down'), whereas ASE accepts only integers. This will work:

structure_dict = {
    'cell':[[2.75,2.75,0],[0,2.75,2.75],[2.75,0,2.75]],
    'pbc': [True,True,True],
    'sites':[
        {
            'symbol':'Si',
            'position':[3/4, 3/4, 3/4],
            'charge': +1,
            'kind_name': 'Si0',
        },
        {
            'symbol':'Si',
            'position':[1/2, 1/2, 1/2],
            'kind_name': 'Si1',
        },
    ],
}

mutable_structure = StructureDataMutable(**structure_dict)
structure = StructureData(**structure_dict)
structure.to_ase().get_tags()

tags being [0,1].

Do you think that we should provide a check for the kind_names? i.e. to provide only the element+integers, or we should automatically change Si_up to Si0? I think we should not allow element+strings, as also in ASE it is not allowed to provide tags as strings.

superstar54 commented 1 month ago

The old StructureData has a mechanism to handle this. https://github.com/aiidateam/aiida-core/blob/1dafdf2ddb38c801d2075d9af9bbde9e0d26c8ca/aiida/orm/nodes/data/structure.py#L2403

However, this makes the user somehow confused, because the kind name is changed, which may affect other input parameters.