nest / nest-simulator

The NEST simulator
http://www.nest-simulator.org
GNU General Public License v2.0
539 stars 365 forks source link

ConnBuilder loop_over_targets() does not work as intended #3106

Open heplesser opened 8 months ago

heplesser commented 8 months ago

Following Ippen et al (2017, Sec 3.2), ConnBuilders should loop over local nodes instead of over targets when the number of local nodes is smaller than the target population. Unfortunately, ever since 20b8f338231e142ab88971a8423b130caaafc2c7, the loop_over_targets() method looks like this (see here in context)

bool
nest::ConnBuilder::loop_over_targets_() const
{
  return targets_->size() < kernel().node_manager.size() or not targets_->is_range()
    or parameters_requiring_skipping_.size() > 0;
}

Here, kernel().node_manager.size() returns the size of the entire network, so the condition will always be true unless targets contains the entire network. This is not what is intended.

I have not checked fully, but I believe we should compare targets.size() to the number of nodes on the pertaining thread.

heplesser commented 7 months ago

@note #3132 will not address this issue, that has to wait for a later PR. For an Ansatz to a solution see 33fa658ac53ab2d65f8e3aceeedf5e0f8ffc671c.

github-actions[bot] commented 5 months ago

Issue automatically marked stale!