Store shells in Neighbors Trajectory

[pmrv]

I've benchmarked this and it's only marginally slower to also compute the shells additionally (maybe ~5%), so I'm always adding this because I want it for my calculations. If anyone thinks it's too much, I will make it configurable whether shells are included or not.

[samwaseda]

shells works only for super perfect structures so I'm not really sure if it ever really plays a role in trajectories

[coveralls]

Pull Request Test Coverage Report for Build 1519730291

• 4 of 6 (66.67%) changed or added relevant lines in 1 file are covered.
• 39 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.07%) to 69.861%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pyiron_atomistics/atomistics/structure/neighbors.py	4	6	66.67%

Files with Coverage Reduction	New Missed Lines	%
pyiron_atomistics/atomistics/structure/atoms.py	1	74.67%
pyiron_atomistics/lammps/base.py	38	79.7%

Totals
Change from base Build 1507820509:	0.07%
Covered Lines:	11453
Relevant Lines:	16394

---

💛 - Coveralls

[pmrv]

I'm using NeighborsTrajectory over in pyiron_contrib also for the TrainingContainer so it's a "trajectory" only in the broadest sense. As I said, though I can put it behind an option.

[sudarsan-surendralal]

Looks good to me!

[samwaseda]

I have a totally different suggestion: How about storing only the vectors and indices, and make distances and shells functions? I am badly concerned by the amount of space this class could accidentally take by mistake (since we're talking about n_snapshots x n_neighbors x n_atoms x n_dim and therefore I'm not super eager to extend the space even more.

[pmrv]

I have a totally different suggestion: How about storing only the vectors and indices, and make distances and shells functions? I am badly concerned by the amount of space this class could accidentally take by mistake (since we're talking about n_snapshots x n_neighbors x n_atoms x n_dim and therefore I'm not super eager to extend the space even more.

I see the issue, but at least for my application we're only talking about MB, so I always want to store this information. How about I put in a keyword argument to enable saving the "derived" data and only store vecs + indices as you proposed if it's not given? How easy is it to instantiate Neighbors to recalculate the data without the starting from the structure?

[samwaseda]

How easy is it to instantiate Neighbors to recalculate the data without the starting from the structure?

I guess this is the real solution that we should aim for, because I have the feeling that it's not Neighbors that's slow, but loading the structures. In situations where the data is so large that calling Neighbors is too costly, it's certainly also imperative to store only the minimum amount of data, meaning most likely only the vectors and the indices.

Realizing all this is unfortunately not quite straightforward in the current format of get_neighbors, which probably I should take on. Since also the user interface will stay the same one way or other, you can also merge this PR as it is right now.