-
Notifications
You must be signed in to change notification settings - Fork 8
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
Redesign of band cuts #64
Comments
After digging a bit in the bandstructure code, I belive my proposal would require a rewriting of the internal function
to something like
The previous functionally with
|
Hi @BacAmorim, thanks for the issue. You might want to check #28 and #52, which are a followup of the discussion we had on #17. Check out the help docstring for A future extension I had planned is to accept a piecewise 1D cut specification in terms of points in the Brillouin zone, so that one could do things like |
Ah, my apologies, I had misunderstood your issue, you are of course using the cut functionality, but have problems with the output ordering. |
My proposal would be to modify the function
We would have two methods for
And the current method, which takes a
This would solve the problem for plotting of band structure along a path, by creating a mesh with k-points,
Edit: typos fixed. |
Of course, the use case with no comesh/cut, would be implemented with the method:
|
This proposal seems to be intended to replace the current approach for I think that requiring to build two meshes is unnecesarily complicated. I did consider this when designing the current If I understand correctly, the core problem you are having is simply the ordering of the vertices. Can you provide a self-contained example? Wouldn't this approach work for your case?
If you need to have each segment have a different length, there are a number of ways to do this, either by having |
That is correct. The problem with your approach is that we would have to define a cut function for each different path. I think that plotting a band structure along a 1D path is a frequent enough task that it deserves a prebuilt solution. As I said before, I also purpose to define a function
and
With these functions and my modification to bandstruture, we could simply do:
|
I think that would be a nice interface, yes! Is there any advantage in |
Some more info: The band extraction algorithm goes a bit crazy when I consider a closed path: (same example as before):
With an open path it words fine:
|
The advantage of a function-based approach is merely for efficiency. If you have a large mesh and comesh, allocating both in memory is redundant, and could be costly. You actually only need memory for one of them. Basically the two meshes in your proposal are just used to encode a mapping (from cut coordinates to BZ coordinates), which is more natural to encode in a function (that's what the mapping is!). Having said that it is completely true that adding support for 1D cuts in terms of points in the API is a good idea. What I don't see the advantage of is requiring the user to build two meshes. The information we need from the user is the sequence of points in the BZ, and perhaps the number of sampling points for each segment. Requiring anything beyond this from the user is unnecessary. This ( |
Hm, interesting, I think I know what is going on here. In general the band-extraction algorithm returns vertices in the order resulting from an adjacent neighbor search, starting from the first vertex in the mesh. If the mesh is compact (i.e. closed), the first vertex is connected to two neighbors, so the search goes both backwards and forwards. This is an unavoidable situation in dimensions greater than 1. But in 1D there is a natural order to the band vertices. In this case we can make sure we sort the vertices according to their lexicographic order, which is trivial to do. For the moment try
|
I moved the closed mesh issue to #67 to keep it separate from the comesh/cutfunc discussion |
Hmm. I was thinking that you might be worried about the size of the mesh and having to duplicate it. I am starting to think that things could be simplified if
No need for that actually. We can just define:
such that the user only writes: |
That refactor would indeed make sense if we decided to use comeshes
In reality I'm most worried about good API design and excessive number of options for the same thing (#19). I'd like to have a conceptually simple
Now, this is a possibility, indeed, but I think it is the wrong abstraction. The cut is the mapping, not the mapped mesh. If we allow to provide an So what is a cut, precisely? It is the function that, applied to the vertices in If we agree on that, the only issue that remains is how to make it easier for the user to build a 1D cut as a polygon through a set of points. The points themselves should be the input, and there should be no need to provide a mesh in this case, unless we want to set its resolution on each segment, its open/close property or whether each segment has an equal length or a length consistent with its original length in the BZ. If we want to support a mesh-free API for 1D cuts we would then need to be able to accept input for all these extra properties. This suggests a possible API like
where I'll try to make a PR hashing out all this soon. |
I see your point. Your idea of using a
Then doing something like
So maybe the general interface shoud be something like
|
Mmm, this is interesting! So, I have two problems with this however
The alternative is to have Please do give this some critical thought @BacAmorim, this kind of input is very valuable. |
So maybe we can define
Then we can use multiple dispatch for the interface:
If we just call We also define:
However, I believe it is better to encode the In this way we don't even have to change the underlying For the particular case of a hamiltonian, if no mesh is provided it can use the default MarchingMesh with predetermied parameters. Or maybe we always force the user to provide a mesh and have a predefined What do you think? |
I'm a bit confused, let me see. So you propose the user never to input a Mesh object, just MeshBuilder objects, which would then produce a Mesh internally? I'm currently working on the linecut branch, where I am experimenting with something similar, but that still keeps
This |
Exactly! A general MeshBuilder would store information to be able to do two things:
I agree. If I understand correctly, this
Now, in truth, it is not necessary to create an aditional |
I have grown to like your unifying proposal, and have drafted #70 along those lines. Please let me know what you think! I'm particularly interested to know if the unified API is sufficiently natural, or feels over-engineered. |
I was trying to implement a way to plot a bandstructure along a 1D path in the Brillouin zone (e.g.: plot the band structure of graphene along the path Gamma -> K -> M -> Gamma as a function of the arclength of the path). I managed to implement a function
linemesh(verts; npts = 20, closed = false)
that given verticesverts
generates a piecewise linear mesh that connects the vertices. I also defined a functionarclength(linepath)
that returns a vector with the arclength measured along the 1D path.So to compute the band structure along the path, I would like to do something like:
Now the problem comes when I want to plot the band structure. Ideally I would like to do something like
The problem lies in the fact that in
vertices(bs, i)
the k points are reordered in a way that is unrelated to the original order inkpath
(as far as I can tell).How could we work arround this?
A possible solution would the to modify
bandstrucure
such that thecut
argument also accepts arrays: such that it would be possible to make:where
kpath
andcut
must have the same length. Therefore, the vertices in the band structure would correspond to he 1D arc mesh and plotting should work with no problems by simply doing:since
[x[1] for x in vertices(bs, 1)]
contains the arclength points.Would this be possible and/or desirable?
The text was updated successfully, but these errors were encountered: