Skip to content
This repository was archived by the owner on Mar 20, 2023. It is now read-only.

Conversation

@kotsaloscv
Copy link
Contributor

No description provided.

@kotsaloscv kotsaloscv self-assigned this Aug 9, 2022
@kotsaloscv kotsaloscv marked this pull request as ready for review August 10, 2022 16:08
@kotsaloscv kotsaloscv requested a review from olupton August 10, 2022 16:08
Copy link
Contributor

@olupton olupton left a comment

Choose a reason for hiding this comment

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

LGTM...I guess you have checked locally the correctness and performance with one/both of 22.5/22.7 (I don't think it's straightforward to test that in the automatic CI)?

I'm also wondering whether this change makes it easy to address

// TODO: check out adding a CellPermute2_CPU version?
and improve the coverage a little there? What do you think?

@kotsaloscv
Copy link
Contributor Author

LGTM...I guess you have checked locally the correctness and performance with one/both of 22.5/22.7 (I don't think it's straightforward to test that in the automatic CI)?

I'm also wondering whether this change makes it easy to address

// TODO: check out adding a CellPermute2_CPU version?

and improve the coverage a little there? What do you think?

Yes it is tested with both 22.5 & 22.7.

Indeed we can include the cell permute 2 CPU version to the test. I will do it.
Thanks for the feedback!

Copy link
Collaborator

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

LGTM.

(discussed some of the questions with Christos offline)

@pramodk pramodk merged commit 4a25a6b into master Aug 12, 2022
@pramodk pramodk deleted the kotsalos/cell_permute_2 branch August 12, 2022 14:23
pramodk pushed a commit to neuronsimulator/nrn that referenced this pull request Nov 2, 2022
…rain/CoreNeuron#847)

* Fixing race condition in cell permute 2 : OpenACC  [performance optimization]
* Update Unit Test / Documentation

CoreNEURON Repo SHA: BlueBrain/CoreNeuron@4a25a6b
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants