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

Parallelize over basis #626

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft

Parallelize over basis #626

wants to merge 10 commits into from

Conversation

MSallermann
Copy link
Member

@MSallermann MSallermann commented Feb 7, 2023

Started to implement parallelization over single spins instead of lattice basis cells.

Closes #420.

Copy link
Member

@GPMueller GPMueller left a comment

Choose a reason for hiding this comment

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

Elegant solution! 👍
Only needs some small changes.

Could use a little refinement and transferral to the C++/OpenMP version, but not necessary for merging IMO

namespace Engine
{

struct Pair_Order
Copy link
Member

Choose a reason for hiding this comment

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

The File should be named like the struct

// Compute the offsets at which each cell atom has to look into the sorted pairs vector
for( int i = 1; i < n_pairs_per_cell_atom.size(); i++ )
{
offset_per_cell_atom[i] += offset_per_cell_atom[i - 1] + n_pairs_per_cell_atom[i - 1];
Copy link
Member

Choose a reason for hiding this comment

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

Do I understand correctly that the resulting intervals of indices into the list of pairs do not overlap, because for the parallel Hamiltonian implementations the lists include the redundant pairs?

for( std::int64_t i = 0; i < n_cell_atoms; ++i )

// This scales quadratically in n_cell_atoms, so we check for co-incident positions only if n_cell_atoms is somewhat small!
if( n_cell_atoms < 100 )
Copy link
Member

Choose a reason for hiding this comment

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

this needs an else with a warning log message telling the user that only the first N atoms were checked

@GPMueller GPMueller linked an issue Feb 17, 2023 that may be closed by this pull request
@muellan muellan force-pushed the develop branch 9 times, most recently from 63fbd74 to 2edff73 Compare June 7, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Core: enable parallelisation over atoms instead of basis cells
2 participants