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

[BBPBGLIB-1145] Write gids without offset in WholeCell LB complexity file #148

Merged
merged 10 commits into from
Apr 15, 2024

Conversation

jorblancoa
Copy link
Collaborator

@jorblancoa jorblancoa commented Apr 4, 2024

Context

Addresses issue highlighted in BBPBGLIB-1145, where the offsets were being calculated before the load balancing, causing issue sometimes when having virtual populations alphabetically ordered before the first real population.

Review

  • PR description is complete
  • Coding style (imports, function length, New functions, classes or files) are good
  • Unit/Scientific test added
  • Updated Readme, in-code, developer documentation

@bbpbuildbot

This comment has been minimized.

@ferdonline
Copy link
Collaborator

Looks promising!

@jorblancoa
Copy link
Collaborator Author

jorblancoa commented Apr 4, 2024

I tested locally the SSCx simulation that had the issue and also launched the long tests, to be sure we don't have the problem found in the past:
https://bbpgitlab.epfl.ch/hpc/sim/blueconfigs/-/pipelines/203550

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@jorblancoa jorblancoa marked this pull request as ready for review April 4, 2024 18:09
@jorblancoa
Copy link
Collaborator Author

Long tests are passing also!
https://bbpgitlab.epfl.ch/hpc/sim/blueconfigs/-/pipelines/203645

WeinaJi
WeinaJi previously approved these changes Apr 5, 2024
Copy link
Collaborator

@WeinaJi WeinaJi left a comment

Choose a reason for hiding this comment

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

Looks good ! Thank you!

Do we need a unit test for this ? I let you decide.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@jorblancoa jorblancoa requested a review from WeinaJi April 9, 2024 09:50
@jorblancoa jorblancoa force-pushed the jblanco/loadbalance_raw_gids branch from a5071b9 to e997630 Compare April 9, 2024 09:51
@jorblancoa jorblancoa changed the title Write gids without offset in WholeCell LB complexity file [BBPBGLIB-1145] Write gids without offset in WholeCell LB complexity file Apr 9, 2024
@bbpbuildbot

This comment has been minimized.

@jorblancoa jorblancoa force-pushed the jblanco/loadbalance_raw_gids branch from 3eb2732 to ad5eab3 Compare April 9, 2024 15:12
@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot

This comment has been minimized.

@bbpbuildbot
Copy link

@jorblancoa jorblancoa merged commit 4dd599e into main Apr 15, 2024
6 checks passed
@jorblancoa jorblancoa deleted the jblanco/loadbalance_raw_gids branch April 15, 2024 14:06
@WeinaJi WeinaJi mentioned this pull request May 8, 2024
4 tasks
WeinaJi added a commit that referenced this pull request May 13, 2024
## Context
After #148, we are writing
raw gids in `Nd.BalanceInfo` instead of final gids with offsets. Such
change is missing in `record_spikes()` leading to no spike written in
the report with NEURON while lb mode is enabled.

## Scope
Fix in the function `cell_distributer.py: record_spikes`.

## Testing
check spikes in `test_loadbal_integration`

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
atemerev pushed a commit that referenced this pull request May 24, 2024
…file (#148)

## Context
Addresses issue highlighted in BBPBGLIB-1145, where the offsets were
being calculated before the load balancing, causing issue sometimes when
having virtual populations alphabetically ordered before the first real
population.

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
atemerev pushed a commit that referenced this pull request May 24, 2024
## Context
After #148, we are writing
raw gids in `Nd.BalanceInfo` instead of final gids with offsets. Such
change is missing in `record_spikes()` leading to no spike written in
the report with NEURON while lb mode is enabled.

## Scope
Fix in the function `cell_distributer.py: record_spikes`.

## Testing
check spikes in `test_loadbal_integration`

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
WeinaJi pushed a commit that referenced this pull request Oct 14, 2024
…file (#148)

## Context
Addresses issue highlighted in BBPBGLIB-1145, where the offsets were
being calculated before the load balancing, causing issue sometimes when
having virtual populations alphabetically ordered before the first real
population.

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
WeinaJi added a commit that referenced this pull request Oct 14, 2024
## Context
After #148, we are writing
raw gids in `Nd.BalanceInfo` instead of final gids with offsets. Such
change is missing in `record_spikes()` leading to no spike written in
the report with NEURON while lb mode is enabled.

## Scope
Fix in the function `cell_distributer.py: record_spikes`.

## Testing
check spikes in `test_loadbal_integration`

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants