Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should there be a distance method? #35

Open
rkurchin opened this issue Jan 3, 2022 · 2 comments
Open

should there be a distance method? #35

rkurchin opened this issue Jan 3, 2022 · 2 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@rkurchin
Copy link
Collaborator

rkurchin commented Jan 3, 2022

I'm imagining a primary syntax something like distance(sys::AbstractSystem, atom_ind_1, atom_ind_2) that would return the distance between two particles in the structure, and maybe also single-index and no-index options that would return distances from one atom to every other one, and a matrix of distances, respectively?

Similarly, should there be a vector version of a similar kind of thing? (i.e. returns the vector in the bounding_box basis rather than just its length)

Personally, I could imagine these having broad enough use (interatomic potentials, geometry optimization, etc.) that it would be worth having, but curious for others' thoughts.

(And of course, as with the beauty of any interface, anyone could dispatch the function on whatever performant method implementation they want)

@rkurchin rkurchin added enhancement New feature or request question Further information is requested labels Jan 3, 2022
@mfherbst
Copy link
Member

mfherbst commented Jan 9, 2022

Hmm. I see your point, but I would advise a bit caution, because this could open up the scene towards a whole zoo of potential methods (what about angles, dihedrals, some projections, spectral/bispectral components, etc.) with the associated explosion in complexity.

I don't have a strong objection, but would like to see more demands first. A key question to me is whether this aspect is so computationally critical that it needs to be part of the interface (allowing for specialisation) as opposed to an add-on package working simply with AtomsBase datastructures in a generic way.

@rkurchin
Copy link
Collaborator Author

rkurchin commented Jan 9, 2022

Yeah, it's a fair point. I suppose I was thinking about it mainly for my own application where I need the distance but not really the other stuff for building graphs, but I see how it might more naturally fit in a separate "helper" package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants