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

In the Grid classes, only save the vectors #69

Merged
merged 3 commits into from
Mar 14, 2022
Merged

Conversation

dpgrote
Copy link
Member

@dpgrote dpgrote commented Mar 14, 2022

In the Grid classes, the user can specify either the individual attributes (such as nx, ny, and nz) or can specify a vector of values (as in number_of_cells). This PR changes the internals so that only the vectors are saved. This avoids the possibility of having the two different values being inconsistent.

This is a change in the internals of PICMI and will affect implementations (it is not backwards compatible). The implementation will need to use the vectors to access the user input parameters.

I note that FBPIC will need to be updated.

First spotted in #60

Note that when changing to the data class scheme, this PR becomes moot.

Comment on lines -271 to -280
self.nx = nx
self.xmin = xmin
self.xmax = xmax
self.bc_xmin = bc_xmin
self.bc_xmax = bc_xmax
self.xmin_particles = xmin_particles
self.xmax_particles = xmax_particles
self.bc_xmin_particles = bc_xmin_particles
self.bc_xmax_particles = bc_xmax_particles

Copy link
Member

@ax3l ax3l Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to add @property for those to make them backwards compatible for users?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarified on slack: backwards compatibility is not needed at this point, we want to move fast and enhance/modernize things

@ax3l ax3l added the enhancement New feature or request label Mar 14, 2022
@ax3l ax3l requested a review from s9105947 March 14, 2022 18:34
@RemiLehe
Copy link
Member

Thanks for this PR @dpgrote. It seems that there is a small conflict ; could you fix it?

@dpgrote dpgrote merged commit 3c9024a into master Mar 14, 2022
@ax3l ax3l deleted the only_store_lists branch March 14, 2022 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants