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

The subtype relation of PlanewaveWavefunction is wrong #193

Open
brainandforce opened this issue Nov 7, 2023 · 0 comments
Open

The subtype relation of PlanewaveWavefunction is wrong #193

brainandforce opened this issue Nov 7, 2023 · 0 comments
Labels
breaking This would be a breaking change

Comments

@brainandforce
Copy link
Owner

Currently, PlanewaveWavefunction{D,T} subtypes AbstractArray{T,D}. This runs into serious problems, primarily with indexing behavior.

I propose that we subtype DenseArray{ReciprocalDataGrid{D,T},3} for the following reasons:

  • DenseArray is a stronger bound than AbstractArray and better reflects the layout of the data. (on a related note, more data structures in this package should probably subtype DenseArray)
  • Indexing a PlanewaveWavefunction{D,T} at a single spin, k-point, and band index should return a ReciprocalSpaceData{D,T}. Indices beyond that reach into the contents to be interpreted as a ReciprocalSpaceData{D,T}, and so have fundamentally different behavior.

Such a change would be breaking (warranting a 0.2 release). I may implement this change as a set of unexported structs/methods before the 0.2 release.

@brainandforce brainandforce added the breaking This would be a breaking change label Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This would be a breaking change
Projects
None yet
Development

No branches or pull requests

1 participant