Implemented atomic representation for a single element type and valence electron only representation

[YAY-C]

• Fix element types ordering (moved to func)
• Fix output name for singlet open-shell
• implemented single atom type rep generation
• Implemented valence-onyl atom density rep
• Fixed argument passing
• Refix argument-passing bug in 5653500

[briling]

We need tests!

[YAY-C]

I added test units for the developped features within test_spahm_a.py.

[YAY-C]

Sorry forgot to push the test-data, tests should be fine now

[briling]

1. I don't get these lines:

        if valence_only:
            a_dm1[start:stop,start:stop] = 0

2. if in test_water_single_element() remove only_z=['O'] the test still passes

[YAY-C]

2. if in test_water_single_element() remove only_z=['O'] the test still passes

I explicitly check for the array shape within the test, that should fix the issue.

[briling]

2. if in test_water_single_element() remove only_z=['O'] the test still passes

I explicitly check for the array shape within the test, that should fix the issue.

cool thanks

[briling]

maybe remove the wrong valence only and have it in another pr? if we even need it

[YAY-C]

maybe remove the wrong valence only and have it in another pr? if we even need it

I moved all the changes for the valence stuff (now in #44),
and kept the single-element stuff in here.

I also added a few changes to get the ecp parameter from the CLI parser.
Should it be in a different PR?

[YAY-C]

I also added a few changes to get the ecp parameter from the CLI parser.

I just realized I should do the same for spahm-b, let me know if you want both in a new PR!

And I was thinking maybe in the main() we should enforce and warn about the need for ecp argument when using minao with elements heavier than Y.
What do you think?

[briling]

maybe this

    for j,i in enumerate(only_i):
        q = mol.elements[i]
        v = rep[j]

can be replaced with this

    for q, v in zip(mol.elements[only_i], rep):

which is easier to read and more similar to the previous version

[briling]

maybe even a one liner

mrep = [np.array((q, v), dtype=object) for q, v in zip(mol.elements[only_i], rep)]

but not sure about readability

[YAY-C]

I liked the one line. but it doesn't work cause mol.elements is a list and not a numpy.array so you can not subscript it with a list of integers.
the intermediate option does not work neither then!

I can create a copy of mol.elements in a numpy.array and go for the oneliner, but it feels a bit like an overhead.
what do you think?

[briling]

i'd put zip(np.array(mol.elements)[only_i], rep)] lol

[briling]

I just don't like j,i

[YAY-C]

ahah sure! Done 👌

[briling]

I just realized I should do the same for spahm-b, let me know if you want both in a new PR!

And I was thinking maybe in the main() we should enforce and warn about the need for ecp argument when using minao with elements heavier than Y. What do you think?

yes to both! but yeah probably this ecp stuff should be separate

[briling]

maybe even a one liner

mrep = [np.array((q, v), dtype=object) for q, v in zip(mol.elements[only_i], rep)]

but not sure about readability

[YAY-C]

• feels like less trouble to leave it like this:

    for j,i in enumerate(only_i):
        q = mol.elements[i]
        v = rep[j]

• and I moved the ECP changes to a new issue/branch Add ecp arguments to main() function and CLI parser #48

[briling]

at this point there's so many commits and reverts that I'd squash and merge rather than merge

[YAY-C]

sinlge-line code

[YAY-C]

ahah sure! Done 👌

[YAY-C]

at this point there's so many commits and reverts that I'd squash and merge rather than merge

agreed, "17 commits" sounds too much for so few changes in the end!

I have never done it though, you wan to do it?