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

What type parameters should AbstractSystem have? #25

Closed
rkurchin opened this issue Nov 17, 2021 · 11 comments
Closed

What type parameters should AbstractSystem have? #25

rkurchin opened this issue Nov 17, 2021 · 11 comments
Labels
question Further information is requested

Comments

@rkurchin
Copy link
Collaborator

Currently we have one D for number of dimensions and one S for the species type. There could be more (e.g. adding Unitful types to specify units for length, velocity, etc.) or fewer, i.e. removing one/both of the current ones. Discuss!

@rkurchin rkurchin added the question Further information is requested label Nov 17, 2021
@rkurchin rkurchin mentioned this issue Nov 17, 2021
@cortner
Copy link
Member

cortner commented Nov 17, 2021

Could somebody quickly recap for me why S is required as a type parameter for an abstract system?

@mfherbst
Copy link
Member

It turned out that keeping the species type parameter made the implementation of an AtomView very hard, so #34 will drop this type parameter, leaving only the dimensionality as type parameter.

@cortner
Copy link
Member

cortner commented May 18, 2024

I need a type parameter for the cell, so I just added another abstract type that has {D, TCELL}. No harm.

The bigger question here is why even D? What does D even mean? If my particle has (position, momentum, mass) is D = 3 or is D = 7? Suppose my particles are electrons and I do VMC so I treat them as a particle system and not a field. is the dimension 3 or 4? (spin) I can keep going ...

Could it become problematic that the D for the particles and the D for the cells need to be the same? It seems to be a bit of a question how broad our audience is. If we restrict ourselves to MD then it seems fine.

@jgreener64
Copy link
Collaborator

It's the number of dimensions of the space the atoms are in. Generally 3 but could be 2 for a simulation of a surface. So usually the D in the SVector{D, T} for each position, though there is freedom in how to store the positions.

One example use is dispatching for visualisation, where 3D and 2D visualisations would use different code paths.

@cortner
Copy link
Member

cortner commented May 19, 2024

of the space the atoms are in

... I understand the intention fine. I was just trying to point out that the interpretation is not quite as clear-cut as one might think the moment we go outside the most basic MD type problems which is exactly what I said above.

Again - I think the way we interpret D right now is fine as long as we keep the audience of the interface limited to those that require point particles that have a position variable living in R^D.

@rkurchin
Copy link
Collaborator Author

I agree with Christoph that strictly there is ambiguity (e.g. for spin as you say) but also agree with Joe that for purposes such as dispatching for visualization, it makes sense to have this. Also, for the position/momenta point, this is still consistent/useful because positions/momenta should be of the same dimensions. Similarly, the length of the PBC vector we've been discussing in #97 should be this too...

@cortner
Copy link
Member

cortner commented May 20, 2024

I think that means we keep what we have now, document clearly what D means and then close the issue?

@rkurchin
Copy link
Collaborator Author

Sounds fine to me – in terms of "document clearly," the docs already say

The D parameter indicates the number of spatial dimensions in the system.

Do you think something beyond this is needed?

@cortner
Copy link
Member

cortner commented May 21, 2024

To my ears this is ambiguous for the reasons I explained above. "Space " for a mathematician and actually many physicists as well is a much broader concept.

@rkurchin
Copy link
Collaborator Author

How about adding a parenthetical along the lines of "(i.e., the dimension of the position vectors)"?

@mfherbst
Copy link
Member

I'll close this due to the merged PR. Feel free to reopen if needed.

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

No branches or pull requests

4 participants