add kinisi.Species and support multiple charged species in ConductivityAnalyzer

[PythonFZ]

• possible fix for ConductivityAnalyzer with multiple species #174
• agree on design
• add tests
• ensure documentation and type checks are passing
• remove debug print statements
• ensure either specie or species_indices can be used

This draft introduces kinisi.Species as an alternative of providing 'specie_indices' as described below

molecules = [[288, 289, 290, 291, 292, 293],
             [284, 295, 296, 297, 298, 299]]
mass    =  [12, 12, 1.008, 1.008, 1.008, 1.008]

params = {'specie': None,
           'time_step': 1.2e-03 * sc.Unit('ps'),
           'step_skip': 100 * sc.Unit('dimensionless'),
           'specie_indices': sc.array(dims=['particle', 'atoms in particle'],
                                      values=molecules,
                                      unit=sc.Unit('dimensionless')),
           'masses': sc.array(dims = ['atoms in particle'],
                              values = mass),
           'progress': False
}

and instead passing

from kinisi import Species

bf4 = Species(indices=[[1, 2, 3, 4, 5], ...], masses=[1, 1, 1, 1, 5], charge=-1)
bmim = Species(indices=[[6, 7, 8, 9, 10], ...], masses=[1, 1, 1, 1, 1], charge=1)

params = {'specie': [bf4, bmim],
           'time_step': 1.2e-03 * sc.Unit('ps'),
           'step_skip': 100 * sc.Unit('dimensionless'),
           'progress': False
}

[arm61]

I am not convinced this is the best approach. The _consolidate_system_particles should be available for both Conductivity and JumpDiffusion (it is useful if you run a really large simulation cell and want to split it up to improve statistics).

[PythonFZ]

I don't quite know how to implement this here. To efficiently consolidate, we would probably have to do this per species type but at this function call, we only have - at best - the charges available to distinguish. Any ideas?

[arm61]

The charge should be optional, with a value of 0 if nothing is given.

[PythonFZ]

I resolved this by changing the default

[arm61]

I need to think a bit about the naming of this.

[arm61]

This is an interesting proposal but a bit of a change from how things work currently. Need to think carefully about how this all works.

[jd15489]

I think this is a really nice idea.
I think it could be very powerful if Species accepted a wider, but limited, range of inputs.

Indices will always need to be a 2d array.
The question is then, does masses need to be a 1d array? It could be a 2d array, but I don't know if we gain from doing that?
Lastly the charges could be a int (as a sc.Variable), but it could also be the per atom charges for the molecule, as a 1d array. kinisi may want a single total charge but the class could handle that.

Thoughts?

[PythonFZ]

I think this is a really nice idea. I think it could be very powerful if Species accepted a wider, but limited, range of inputs.

Thank you!

Indices will always need to be a 2d array. The question is then, does masses need to be a 1d array? It could be a 2d array, but I don't know if we gain from doing that? Lastly the charges could be a int (as a sc.Variable), but it could also be the per atom charges for the molecule, as a 1d array. kinisi may want a single total charge but the class could handle that.

Adding masses as 2D array could allow for different ordering of the atom indices inside each molecule. Currently, I think each molecule needs to have the same order, e.g. H, H, O and O, H, H would not work, because the weights would be assigned incorrectly (not sure if overall the movement of the COM would be consistent, but I don't think so?). But this would be a more complex to implement and maybe beyond this PR?

For more complex features, I am mostly working with liquids and I am using SMILES / SMARTS to identify the indices (shameless advertisement https://zincware.github.io/rdkit2ase/) so one could, instead of indices allow for a SMILES string to identify molecules. But I am not sure if this fits nicely with the package and personally, I think it would fit better to keep things seperate.

[jd15489]

Adding masses as 2D array could allow for different ordering of the atom indices inside each molecule. Currently, I think each molecule needs to have the same order, e.g. H, H, O and O, H, H would not work, because the weights would be assigned incorrectly (not sure if overall the movement of the COM would be consistent, but I don't think so?). But this would be a more complex to implement and maybe beyond this PR?

For more complex features, I am mostly working with liquids and I am using SMILES / SMARTS to identify the indices (shameless advertisement https://zincware.github.io/rdkit2ase/) so one could, instead of indices allow for a SMILES string to identify molecules. But I am not sure if this fits nicely with the package and personally, I think it would fit better to keep things seperate.

I agree.

I still think it would be useful to be able to specify the per atom charges though, in the same way as masses.

[PythonFZ]

Being able to define charges per atom would be nice. Of course, charge distributions will fluctuate over the course of an MD, but using e.g. the mean could give e.g. better estimates of the center of the dipole than just using the center of mass.

I don't have the deep code knowledge of the package that you have and all these changes would require me to spend more time on this PR than I currently have.

[arm61]

I am conscious that we need to be clear on how kinisi works for the centre of mass/charge style displacement things. I will try to address this in the jump diffusion documentation notebook now and open a PR.

[arm61]

@PythonFZ, does the notebook as a part of #183 clarify how the jump diffusion (and therefore the conductivity) analyzer stuff works?

[arm61]

@PythonFZ, we really like this idea. But I am concious that @jd15489 has some nice ideas for it. Would you be interested in having a zoom call at somepoint to discuss?