[BG fields] Add refactored TWTS laser by BeyondEspresso · Pull Request #704 · ComputationalRadiationPhysics/picongpu

BeyondEspresso · 2015-02-13T15:53:30Z

By re-implementing the TWTS laser as functors, this request concludes a series based on the original pull request #597. Refactoring its separate features led to the subsequent pull requests #600 (BG Fields in mySimulation.hpp, merged), the #664 (Complex class in libPMacc, open) and now this one.

Compared to Add twts laser #597, the implementation also includes the correct back-rotation of vector components on the staggered components of the field grids, improved support for 2D and .
Functionality tests with and without fields show the same results than a parallel implementation without functors, which directly proceeded Add twts laser #597.
Performance tests show a lower performance (~10% without particles and currents, ~1% with particles and currents) than the reference implementation. This performance probably arises from an increased number of float_64 variables. This penalty in performance is currently acceptable.
The full sources and documentation of the equations and program that generated the core code for the fields are now at Klaus for cross-checking. However, since this chross-check is done independently and will take some time (probably ~1 month), the focus of this pull request will be more on the implementation aspects.
The code includes the current unmerged status of Close #608 Complex Math in libPMacc #664.
Future: This code will be updated as soon as the const expression feature becomes available in CUDA and PIConGPU. The aim of this update will be improved speed and a further generalization of the field rotation routines as functors for general use in PIConGPU/libPMacc.

Happy code ripping! 😋

To Do

rebase against dev as soon as Close #608 Complex Math in libPMacc #664 is merged

BeyondEspresso · 2015-02-13T16:25:41Z

The 2D case is currently not passing.

ax3l · 2015-02-13T16:50:42Z

thank you for the pull request 👍
looks nice already - great work. We will do a detailed review next week ✨

Have a good weekend! :)

BeyondEspresso · 2015-02-13T23:33:31Z

Above fix is very dirty 🙈 . Suggestions on improvements, without resorting to pre-compiler directives, are welcome 😇 .

bussmann · 2015-02-13T23:41:52Z

Nice!

ax3l · 2015-02-17T01:00:53Z

it's still a bit hard to review due to the missing rebase against the merged complex pull and the huge number of single commits.

I will try a first swipe, but no guarantee :)

ax3l · 2015-02-17T01:03:05Z

since you return the result in SI pls use float3_64 here, too.

Since the result is a normalized quantity the datatype is correct, but I renamed the function.

makes sense, well done.

BeyondEspresso · 2015-02-20T18:49:22Z

Pull request repository is rebased. (Not just rebased, but the history is cleaner now.)

BeyondEspresso · 2015-03-02T17:42:12Z

In addition to your suggestions I further cleaned up code and interface. Now the code precision can be changed on an individual basis and the coordinate normalization should make it possible now to use the speedier float_X datatype.

Since this round of refactoring has now ended, please go ahead with the review and hack this to pieces! 😄

ax3l · 2015-03-03T10:39:34Z

sounds great, we will have a look! ✨

ax3l · 2015-03-03T10:43:43Z

@BeyondEspresso you have become a very gentle developer in a steep curve, I like that! Nice and clean. 👍

psychocoderHPC · 2015-03-17T09:25:26Z

please use PMACC_ALIGN

BeyondEspresso · 2015-03-17T10:56:50Z

@psychocoderHPC 's suggestions are now online.

ax3l · 2015-03-20T00:26:54Z

small reminder: spaces around operators increase readability (and are part of our coding guidelines).

BeyondEspresso · 2015-03-20T11:05:45Z

Fixed this.
Wrote #766 as constructive criticism on the "spacings around division operator coding guideline".

ax3l · 2015-03-24T01:28:22Z

@BeyondEspresso great work!

@psychocoderHPC since this is a bigger pull request and I think it is ready for a merge for now - are you d'accord? :)

ax3l · 2015-03-24T05:47:52Z

field-vectors ;)

@ax3l Note that this is currently unique code that applies to the B-field only (because of intra-cell position offsets). My intention is to generalize this in the upcoming feature update when I introduce different polarizations for the TWTS laser field. (Update: I understand now you were correcting a spelling mistake. 👻 )

BeyondEspresso · 2015-03-25T14:55:42Z

Ok, I found and fixed another bug in the 2D section. In addition I did some more polishing on the comments. Let's merge this now. 😉

BeyondEspresso · 2015-03-25T15:59:48Z

Bad bugs kick the bucket last...

ax3l · 2015-03-26T16:03:06Z

merge-able now? ;)

…mplex-BeyondEspresso [BG fields] Add refactored TWTS laser

ax3l added this to the Open Beta milestone Feb 13, 2015

ax3l added feature component: core in PIConGPU (core application) labels Feb 13, 2015

ax3l self-assigned this Feb 13, 2015

ax3l reviewed Feb 17, 2015
View reviewed changes

BeyondEspresso force-pushed the topic-redoTWTS-libPMaccComplex-BeyondEspresso branch from 7ebe4b9 to cfbe35a Compare February 20, 2015 18:42

BeyondEspresso force-pushed the topic-redoTWTS-libPMaccComplex-BeyondEspresso branch from a715496 to 264b416 Compare February 27, 2015 12:23

ax3l reviewed Mar 3, 2015
View reviewed changes

psychocoderHPC reviewed Mar 17, 2015
View reviewed changes

Include @psychocoderHPC review suggestions.

bafe7f2

Apply more code styling. Lather, rinse, repeat!

7477e0b

ax3l removed the wait-4-author/WIP label Mar 20, 2015

ax3l reviewed Mar 24, 2015
View reviewed changes

ax3l added the wait-4-author/WIP label Mar 24, 2015

BeyondEspresso added 2 commits March 25, 2015 14:38

Bugfix in 2D and comments.

954e61e

Improve comments.

2aa5941

Fixed bug in 2D B-field.

f22f343

ax3l removed the wait-4-author/WIP label Mar 26, 2015

ax3l added a commit that referenced this pull request Mar 26, 2015

Merge pull request #704 from BeyondEspresso/topic-redoTWTS-libPMaccCo…

bb6f295

…mplex-BeyondEspresso [BG fields] Add refactored TWTS laser

ax3l merged commit bb6f295 into ComputationalRadiationPhysics:dev Mar 26, 2015

Conversation

BeyondEspresso commented Feb 13, 2015

To Do

Uh oh!

BeyondEspresso commented Feb 13, 2015

Uh oh!

ax3l commented Feb 13, 2015

Uh oh!

BeyondEspresso commented Feb 13, 2015

Uh oh!

bussmann commented Feb 13, 2015

Uh oh!

ax3l commented Feb 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeyondEspresso commented Feb 20, 2015

Uh oh!

BeyondEspresso commented Mar 2, 2015

Uh oh!

ax3l commented Mar 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeyondEspresso commented Mar 17, 2015

Uh oh!

ax3l commented Mar 20, 2015

Uh oh!

BeyondEspresso commented Mar 20, 2015

Uh oh!

ax3l commented Mar 24, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeyondEspresso commented Mar 25, 2015

Uh oh!

BeyondEspresso commented Mar 25, 2015

Uh oh!

ax3l commented Mar 26, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants