Skip to content

Downsize the C++ Structure class

Magnus Rahm requested to merge downsize-cpp-Structure-class into master

This is for partial completion of #551 (closed).

Closes #458 (closed)

It turns out the C++ class (and its PyBind interface) was unnecessarily large, so with this MR we get rid of quite a bit of code:

  • I removed an unused overloaded function (set_number_of_allowed_species(int))
  • There was a variable uniqueSites together with several functions, which were only used by tests and by the Cluster class. In the Cluster class, its usage was trivial and never used by anyone else, so I removed it altogether.
  • The Structure class juggled both chemical symbols and atomic numbers. I think that is a Python-side job, so now it only knows about atomic_numbers. This means the user interface to Structure is changed, but I don't think anyone used it explicitly anyway. (It still seems backwards compatible.)
  • There were two very similar functions that both were exposed to the Python side, findLatticeSiteByPosition and findLatticeSiteByPositions. Since the latter was just a loop over the former, I made it a Python function. To do this I had to rewrite Structure.py somewhat. This should in principle be fine, but I had to add one slight WTF. The Structure class now inherits from _Structure, similar to ClusterSpace and OrbitList. The difference is that sometimes you get _Structure objects from C++, instead of Structure. As a user you will probably never notice, because they are always converted to ASE Atoms, but internally it is a little bit strange. This is the price of a simpler PyBind interface and less C++ code, but I'm willing to revert if you want.
Edited by Magnus Rahm

Merge request reports