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.

Sign up for GitHub

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

Jump to bottom

Remove attribute chargeState, add boundElectrons #706

Merged
merged 2 commits into from
Feb 18, 2015
Merged

Remove attribute chargeState, add boundElectrons #706

merged 2 commits into from
Feb 18, 2015

Conversation

n01r
Copy link
Member

@n01r n01r commented Feb 16, 2015

In this PR the following has been refactored:

  • remove attribute chargeState, add boundElectrons
  • add trait GetChargeState.hpp
  • replace all uses of chargeState with boundElectrons

@n01r
Copy link
Member Author

n01r commented Feb 16, 2015

I still need a way to connect the default value of boundElectrons to be connected to the proton number of the species. I have a feeling that this is not possible in speciesAttributes.param. Probably one should look at ParticlesInit.kernel for this.

@ax3l ax3l added feature component: core in PIConGPU (core application) labels Feb 16, 2015
@ax3l ax3l added this to the Open Beta milestone Feb 16, 2015
@ax3l ax3l self-assigned this Feb 16, 2015
ax3l
ax3l reviewed Feb 17, 2015
View reviewed changes
src/picongpu/include/simulation_defines/param/speciesAttributes.param
* - \todo calculate from new attribute boundElectrons */
value_identifier(float_X,chargeState,float_X(1.0));
*
* \todo connect default to proton number
Copy link
Member

Choose a reason for hiding this comment

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

yes, pls :)

Copy link
Member Author

Choose a reason for hiding this comment

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

waiting for @psychocoderHPC 's #640

@ax3l
Copy link
Member

ax3l commented Feb 17, 2015

looks already pretty clean 👍

I am not sure about the getChargeState fallback/error and the minor that we have not seen last time with the units. can be checked tomorrow. everything else is inline :)

@n01r n01r force-pushed the topic-attributeBoundElectrons branch from f8f1dc3 to 5793929 Compare February 17, 2015 15:19
@n01r
Copy link
Member Author

n01r commented Feb 17, 2015

I rebased ... where is dat merge conflict

Marco Garten added 2 commits February 17, 2015 17:43
Remove attribute chargeState, add boundElectrons
68b8f27 
In this commit the following has been refactored:
    -- remove attribute `chargeState`, add `boundElectrons`
    -- add trait `GetChargeState.hpp`
    -- replace all uses of `chargeState` with `boundElectrons`
Doxygen comments / set boundElectrons default to 0
007d765
@n01r n01r force-pushed the topic-attributeBoundElectrons branch from 5793929 to 007d765 Compare February 17, 2015 16:43
@ax3l
Copy link
Member

ax3l commented Feb 17, 2015

-> github clone/fetch issues. travis is suffering, too.

ax3l added a commit that referenced this pull request Feb 18, 2015
@ax3l
Merge pull request #706 from n01r/topic-attributeBoundElectrons
668cf67 
Remove attribute chargeState, add boundElectrons
@ax3l ax3l merged commit 668cf67 into ComputationalRadiationPhysics:dev Feb 18, 2015
@n01r n01r deleted the topic-attributeBoundElectrons branch February 18, 2015 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ax3l ax3l

Labels
component: core in PIConGPU (core application)
Projects
None yet
Milestone
0.2.0: Open Beta
Development

Successfully merging this pull request may close these issues.
3 participants
@n01r @ax3l @psychocoderHPC