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

remove iterators and replace with for_each, parallel_for_each, parallel_reduction, ... #434

Closed
SteffenSeckler opened this issue Mar 26, 2020 · 13 comments · Fixed by #626
Closed
Assignees
Labels
kokkos everything related to usage of kokkos

Comments

@SteffenSeckler
Copy link
Member

SteffenSeckler commented Mar 26, 2020

Is your feature request related to a problem? Please describe.
Needed for proper kokkos support.
Also makes our lives easier

@SteffenSeckler
Copy link
Member Author

SteffenSeckler commented Mar 27, 2020

particle deletion will probably become more complicated that way and probably needs an additional method, e.g., autopas::delete_if() or so.
This would also solve #435.

@SteffenSeckler
Copy link
Member Author

or, probably also needed for the LC-reference-cell container (tentative name) just flag a particle as deleted and delete them only later.

@l-gaertner
Copy link
Contributor

I will add a parallel_for_each in a future commit for #626
This will however just be calling the sequential for_each, until kokkos is integrated.

Additionally, examples (eg. md-flex) will be adapted in a future mr.

@FG-TUM
Copy link
Member

FG-TUM commented Aug 25, 2021

I would actually like to use _parallel as a suffix in all of these names.

@l-gaertner
Copy link
Contributor

I would actually like to use _parallel as a suffix in all of these names.

Camel cased or with underscore?

so forEachParallel or forEach_parallel ?

@FG-TUM
Copy link
Member

FG-TUM commented Aug 25, 2021

Right, probably forEachParallel for consistency ;)

@l-gaertner
Copy link
Contributor

On the topic of reductions:
Is there a necessity for "region reductions"?
Or are reductions currently (and in the future) always performed on all particles (still considering IteratorBehavior).

@FG-TUM
Copy link
Member

FG-TUM commented Aug 25, 2021

Good question. I would assume reductions only of a subset of particles are also of interest.

@l-gaertner
Copy link
Contributor

l-gaertner commented Aug 25, 2021

I know how to trigger your sense of creating Arbeitsbeschaffungsmaßnahmen and should probably stop right? xD

@l-gaertner
Copy link
Contributor

l-gaertner commented Aug 25, 2021

Should we start to consider templating these?
Parallel and region forEaches and reducers [and const versions for both] start to generate a lot of code.
At least internally, for the public api this is probably a bad idea.

@FG-TUM
Copy link
Member

FG-TUM commented Aug 25, 2021

I'd say just start implementing them, and when you notice that there is a huge overlap of code try to remove redundancies e.g. via templates or calls to helper functions.

@SteffenSeckler
Copy link
Member Author

question: how are parallel foreachs named in kokkos or other codes? does it maybe make sense to use a similar naming scheme?

@l-gaertner
Copy link
Contributor

l-gaertner commented Aug 25, 2021

question: how are parallel foreachs named in kokkos or other codes? does it maybe make sense to use a similar naming scheme?

parallel_for and parallel_reduce
https://github.com/kokkos/kokkos/wiki/ParallelDispatch

I would honestly keep it consistent with the rest of AutoPas.

@l-gaertner l-gaertner self-assigned this Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kokkos everything related to usage of kokkos
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants