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

Fix #382 Phase Space Moving Window #391

Merged
merged 3 commits into from
May 2, 2014

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented May 1, 2014

Due to ComputationalRadiationPhysics/libSplash#127 the used interface did not reflect the new position
of the GPU in the output. The interface proposed in this pull request saves an MPI_Allgather and corrects that issue (see #382).

Internal namings have been updated to fit the new conventions from #128 (comment) .

The meta information for the moving window offset has been added (but this pull does not yet add cropping as planned in #89).

ax3l added 3 commits May 1, 2014 16:50
Due to ComputationalRadiationPhysics/libSplash#127
the interface used did not reflect the new position
of the GPU in the output.
The interfaced used now saves an MPI_Allgather and
corrects that issue.
Adds the offset caused by the moving window for the y-direction.
Cropping to the moving window would be better and will follow late on.
@ax3l ax3l added bug labels May 1, 2014
@ax3l ax3l added this to the Open Beta milestone May 1, 2014
@f-schmitt
Copy link
Member

Please note that the used interface did not reflect the new position as this is its intended behavior. After all, your MPI position/rank did not change.

@ax3l
Copy link
Member Author

ax3l commented May 2, 2014

But isn't the call to fAttr.mpiPosition.set(...) exactly that MPI
position?

Please note that the used interface did not reflect the new position as
this is its intended behavior. After all, your MPI position did not change.


Reply to this email directly or view it on GitHub
#391 (comment).

@f-schmitt
Copy link
Member

Unfortunately not. For PDC, everything except the file access type if ignored from FileCreationAttr. This is probably not the best idea but noted in the usermanual.pdf.

@ax3l
Copy link
Member Author

ax3l commented May 2, 2014

See cross post in ComputationalRadiationPhysics/libSplash#127 (comment) - I must have overlooked that this option has no effect for (yet) :)

DataCollector::FileCreationAttr fAttr;
Dimensions mpiPosition( gc.getPosition()[axis_element.first], 0, 0 );
Copy link
Member

Choose a reason for hiding this comment

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

I can merge this but maybe you want to remove setting the mpiPosition beforehand?

Copy link
Member Author

Choose a reason for hiding this comment

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

since I am explicitly setting the offsets it won't get wrong when the option gets available I guess.
so we could leave it in for now - makes the function a bit longer but maybe more clear "who is where".

f-schmitt pushed a commit that referenced this pull request May 2, 2014
Fix #382 Phase Space Moving Window
@f-schmitt f-schmitt merged commit 952903a into ComputationalRadiationPhysics:dev May 2, 2014
@ax3l ax3l deleted the fix-psMoving branch May 2, 2014 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the project's code component: plugin in PIConGPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants