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

Boundary Conditions #97

Closed
Tracked by #101
cortner opened this issue May 13, 2024 · 8 comments
Closed
Tracked by #101

Boundary Conditions #97

cortner opened this issue May 13, 2024 · 8 comments

Comments

@cortner
Copy link
Member

cortner commented May 13, 2024

PROPOSAL

Replace

boundary_conditions::SVector{D, BoundaryCondition}

with one of

# option 1  (strong preference for this)
pbc::NTuple{D, Bool}

# option 2
struct PBC{D}
   vals::NTuple{D, Bool}
end 

boundary_condition::PBC{D}

and to clarify in the documentation the difference between boundary conditions and constraints.

This would be a breaking change and affect downstream packages.

ORIGINAL POST:

The boundary conditions currently cannot take two values without type instability:

boundary_conditions::SVector{D, BoundaryCondition}

If I want PBC in x, y but open in z direction? A possible solution is to allow Periodic(true) and Periodic(false). But e.g. Periodic(), Open() will not work. As I mentioned on Zulip, I think for particle systems there is only one type of boundary condition: periodic {true, false}. Everything else is to be implemented as a contraint. I propose to remove boundary_conditions entirely and replace it by a pbc::NTuple{D, Bool}.

ZeroDirichlet or anythig like that should be implemented as a contraint, not a boundary conditions. (this is different from continuum mechanics)

There are some more complicated cases that I'm not 100% certain how to address. E.g. reflections. (quadrupole method) but I suspect those are easiest to implement by just storing a larger system + the information that this systems has a certain symmetry

@rkurchin
Copy link
Collaborator

This sounds fine to me, but I would want to get thoughts from folks who do other kinds of simulations in case there's something I'm not thinking of.

Conceptually, I think there may well be potential users who are not crystal clear about the distinction between BC's and constraints (this claim is based on some colloquial/sloppy language regarding these ideas that I certainly hear with some frequency), so whatever we settle on, we should add some clear language in both comments and documentation.

@cortner
Copy link
Member Author

cortner commented May 15, 2024

A comment from @mfherbst was that ZeroDirichlet is used for electronic structure. I agree this makes sense there, but I pointed out that that's a boundary condition for the electronic fields and not part of the particle system. My strong preference is to keep the particle system just a particle system and to not incorporate other properties into it.

@cortner
Copy link
Member Author

cortner commented May 15, 2024

there may well be potential users who are not crystal clear about the distinction between BC's and constraints

This is certainly true. I use the term boundary condition myself in my papers for things that I implement as constraints. We could avoid the term "boundary condition" entirely and call it something else. But that may be equally confusing.

My personal preference is to call it "periodic boundary condition" (true or false) and document how other kinds of constraints are implemented.

@cortner
Copy link
Member Author

cortner commented May 15, 2024

I've edited the original post to make a concrete proposal. I think this is important to do correctly. And I'd like to do this soon since it is one of the things that prevents me from retiring JuLIP.

CC @mfherbst @rkurchin @jgreener64 @jameskermode

Simple examples that require this: graphene sheet, TBG etc, nanotubes, nanowires, cracks or dislocation lines with clamped "boundary conditions" (constraints!!) in xz, periodicity in z, ...

@cortner
Copy link
Member Author

cortner commented May 15, 2024

I can mention that in JuLIP I had a DOF manager that implemented masks (a generalization of clamped position) and things like volume constraints and that extracted a DOF vector from the set of positions and cell. This could be added here again .

This was referenced May 15, 2024
@mfherbst
Copy link
Member

mfherbst commented May 16, 2024

As I mentioned on zoom, I think avoiding to muddle up physical structure and modelling details makes a lot of sense.

But in line with that I would not call the field PBC (periodic boundary conditions), since BC then suggests the wrong things in my head. How about just periodic::NTuple{D, Bool} ? Or periodicity:: ...

@jameskermode
Copy link

I would support this change. Calling the periodicity 3-vector periodic is ok with me. The advantage of pbc is that this is the name used in ASE and ExtXYZ, but we could convert this when reading/writing.

@cortner
Copy link
Member Author

cortner commented May 16, 2024

The case for pbc is that it is the acronym used by literally everybody. The case for periodic is that it avoids the usage of "boundary condition" which - for a particle system - is arguable not a great term. Personally, I would go with convention i.e. pbc

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

No branches or pull requests

4 participants