Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Improve mem. ops. + NodeID-ElementID mapping bindings#179

Merged
mgeplf merged 2 commits intomasterfrom
sandbox/srivas/memory_improvements
Feb 21, 2022
Merged

Improve mem. ops. + NodeID-ElementID mapping bindings#179
mgeplf merged 2 commits intomasterfrom
sandbox/srivas/memory_improvements

Conversation

@sergiorg-hpc
Copy link
Contributor

@sergiorg-hpc sergiorg-hpc commented Feb 18, 2022

The PR improves the copy of the returned ids when fetching the data from the reports, as well as the Python bindings related to the NodeID-ElementID mapping (#168):

  • In the former case, the std::vector is swapped in a constant-time operation instead of a full copy. This is particularly useful when a large number of GIDs is requested.
  • In the latter case, the binding of the NodeID-ElementID mapping from is improved, returning a numpy.array instead of a list of tuples. Hence, providing directly the result generated from the C++ code without any additional copies.

Using the same ~60TB report from #174 that contains 4.2M GIDs (1.5B compartments), the following table illustrates the performance obtained by fetching a single timestep and varying the number of GIDs requested:

 # GIDS |            v0.1.11 |                 PR
 ------- -------------------- -------------------
      1 |  0.010879278182983 |  0.001698255538940
     10 |  5.343438386917114 |  5.512796163558960
    100 |  5.943392753601074 |  5.604890823364258
   1000 |  6.406604766845703 |  6.062043905258179
  10000 |  6.451685428619385 |  6.049137830734253
 100000 |  7.101217269897461 |  6.462664365768433
1000000 | 12.762017250061035 |  9.955524444580078
4234929 | 27.868659019470215 | 18.158316850662230

On the other hand, when using in Python the function get_node_id_element_id_mapping() from #168, the following table shows the performance obtained varying the number of GIDs requested:

 # GIDS |            v0.1.11 |                 PR
 ------- -------------------- -------------------
      1 |    0.0158734321594 |  0.001111268997192
     10 |    2.6325545310974 |  2.509858608245849
    100 |    2.9563860893249 |  2.782548427581787
   1000 |    3.2792141437530 |  2.988783836364746
  10000 |    5.4664847850799 |  3.832525491714477
 100000 |   24.0672097206115 |  3.506523370742798
1000000 |  225.1766004562378 |  6.213826656341553
4234929 | 1017.6884262561798 | 11.914409875869751

Note that there has been other attempts to improve the copies of the voltages and even the tuples for the NodeID-ElementID mapping container, but the benefits turned out to be negligible compared to the source code complexity that was required.

@sergiorg-hpc sergiorg-hpc self-assigned this Feb 18, 2022
@sergiorg-hpc sergiorg-hpc requested review from alkino, jorblancoa, matz-e and mgeplf and removed request for matz-e February 18, 2022 16:43
@sergiorg-hpc sergiorg-hpc requested review from matz-e and removed request for jorblancoa February 18, 2022 16:45
@sergiorg-hpc sergiorg-hpc force-pushed the sandbox/srivas/memory_improvements branch 2 times, most recently from 7f7f68a to 342b33b Compare February 18, 2022 16:53
alkino
alkino previously approved these changes Feb 18, 2022

// Fill ids
data_frame.ids = node_id_element_layout.ids;
data_frame.ids.swap(node_id_element_layout.ids);
Copy link
Contributor

Choose a reason for hiding this comment

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

it's funny, I had this, but the asm turned out the same in both cases; now I'm doubting that.

@mgeplf mgeplf merged commit f74333b into master Feb 21, 2022
@mgeplf mgeplf deleted the sandbox/srivas/memory_improvements branch February 21, 2022 07:54
@sergiorg-hpc
Copy link
Contributor Author

Thank you very much, @matz-e and @mgeplf!

@mgeplf
Copy link
Contributor

mgeplf commented Feb 21, 2022

Thanks for the change

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