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

add positions_relative or similar? #124

Open
rkurchin opened this issue Oct 23, 2024 · 8 comments
Open

add positions_relative or similar? #124

rkurchin opened this issue Oct 23, 2024 · 8 comments
Labels
question Further information is requested

Comments

@rkurchin
Copy link
Collaborator

The workshop discussion on dimensionless systems yesterday was inconclusive at best, but a related thing that occurred to me that might also at least partially address the whole Unitful + AD potential snarl while hopefully avoiding some of the ambiguity issues we're concerned about could be an explicit way to get relative coordinates of particles (i.e. in terms of the lattice vectors of the bounding box).

What do people think about this? And if we were to add it, do folks have thoughts about whether it should be a separate function vs. e.g. a separate dispatch of the positions function, e.g. via a flag that could be passed or something?

@rkurchin rkurchin added the question Further information is requested label Oct 23, 2024
@jgreener64
Copy link
Collaborator

As I understand it you mean fractional coordinates? I could see the advantage of having that, especially since there is a sensible fallback.

@rkurchin
Copy link
Collaborator Author

Yes, that's what I meant, thanks for clarifying!

@cortner
Copy link
Member

cortner commented Oct 23, 2024

I have no opinion on introducing a relative coordinates interface. I can see it being useful.

But I don't think this solves the dimensionless systems request, it would feel like a hack - using a framework outside of the scope for which it was intended - with unpredictable consequences.

@mfherbst
Copy link
Member

I think the main argument that has been put on the table against this has always been: What to do for isolated systems (cell isa IsolatedCell) ? Fractional coords don't really make sense here. I'm not sure how to handle that in a generic interface while keeping a flexibility for the cell.

@cortner
Copy link
Member

cortner commented Oct 24, 2024

throw an error

@mfherbst
Copy link
Member

Ok, but my point is if this is part of the interface and I write my package against it than implicitly I am assuming that the cell is PeriodicCell. So should this not be made explicit ?

@rkurchin
Copy link
Collaborator Author

Now we go back down the proliferating type parameters rabbit hole I suppose...though I could see an argument for it in this case...

@cortner
Copy link
Member

cortner commented Oct 24, 2024

Just use PeriodicCell(FFF). I don't really understand the need for the IsolatedCell anyhow.

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