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.

Sign up for GitHub

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

Jump to bottom

Mech ABI: The final step. #1452

Merged
merged 110 commits into from
Jul 29, 2021
Merged

Mech ABI: The final step. #1452

merged 110 commits into from
Jul 29, 2021

Conversation

thorstenhater
Copy link
Contributor

@thorstenhater thorstenhater commented Mar 22, 2021 •
edited
Loading

Status Complete Review #1

  1. This is the final stage of the ABI proposal, as outlined in Add Mechanism ABI #1376.
  2. Review turned up the request for further simplification
  • Remove concrete_mechanism<Backend B>
  • Make mechanism a concrete type
  • Remove typeid from catalogues

Note Please read the docs first.

  • Add a public, plain-C (C11) (C99) compilable header for extensions to interface with.
  • Separate mechanisms into parts: one implementing the interface and one in arbor wrapping the interface type.
  • Document said interface.
    • Inline in header
    • Internal: extend mechanism catalogues
  • Adjust modcc to use this interface.
    • CPU/Scalar
    • CPU/SIMD
    • GPU
  • Test implementation.
    • CPU/Scalar
    • CPU/SIMD
    • GPU
  • Adjust unit tests.
    • modcc: output
    • rest: new API
    • Add unit tests
      • Memory initialisation
        • CPU
        • GPU
  • Adjust scripts/build_catalogue to insulate from arbor source tree.
    • Fix up docs.
  • Polish pass
    • See if we can unify mechanism CPU/GPU. No, memory abstraction is too weak.
    • See if mechcat can just store the interface type and produce mechanisms on. Postponed until after this PR.
    • Hoist methods from headers: done where possible
    • clean-up install

Also, this makes the following incidental changes

  • Add fmt for formatting in modc.
  • Fix some -pedantic warnings in files touched by the PR.
  • Early return in stimulus GPU kernel; avoids launch failure
  • Fix rev_pot mech classification

Closes #1376
Closes #1335
Closes #1590

@thorstenhater thorstenhater marked this pull request as draft March 22, 2021 13:37
@bcumming
Copy link
Member

Is there a specific reason for C11 for the API?

If possible, it is best to stick to ANSI C99, for maximum portability.

@thorstenhater
Copy link
Contributor Author

Just the the dream of C++20, which will be aligned with C11. But good point, I'll change that to C99.

@thorstenhater
Scalar compiles, passes all tests.
dee9cc5
@thorstenhater
Merge master, pass again.
dc9a132
@thorstenhater
Make underscore situation C++ conform.
a498fb3
@thorstenhater
Revert change on constraint_partition; SIMD compiles.
d11c8bf
@thorstenhater
SIMD passes all tests.
bf57ad3
@thorstenhater
Add FMT properly.
58fe357
@thorstenhater
Streamline GPU/CPU gap. Make GPU first steps.
f71cc9d
@thorstenhater
Excise net_receive. GPU prep-work.
56f3ebc
@thorstenhater
Excise net_receive. GPU prep-work.
5fa9bf9
@thorstenhater
GPU prep-work complete.
f64972c
@thorstenhater
Fix gpuprinter.
9cea552
@thorstenhater
warnings.
bd7f325
@thorstenhater
Compiling CPU, SIMD, and GPU. Now on to linking.
c4943ce
@thorstenhater
Fix Cmake some more.
2b6c42b
@thorstenhater
Fix Cmake some more.
c5ddc82
@thorstenhater
Linking.
7ed066c
@thorstenhater
GPU+CPU compile
0026918
@thorstenhater
Clean up
b482f93
@thorstenhater
GPU prep-work complete.
44da04e
@thorstenhater
Merge.
b600f50
@thorstenhater
Remove mech_info: SIMD+Scalar pass all tests.
54846e5
@thorstenhater
GPU compiles, links.
b3b622f
@thorstenhater
Interface for mechcat, get rid of mechanismKind.
f814599
@thorstenhater
Merge branch 'feature/mech-abi-final' of github.com:thorstenhater/arb…
b8243de 
…or into feature/mech-abi-final
@thorstenhater
Adjust for GPU.
e9f5d62
thorstenhater and others added 11 commits July 2, 2021 09:55
@thorstenhater
Review #2.
2cf1ba2
@thorstenhater
Merge branch 'feature/mech-abi-final' of github.com:thorstenhater/arb…
c3d7214 
…or into feature/mech-abi-final
@thorstenhater
Fix most review comments, tests left open.
a5d8105
@halfflat
wip
1d2c0d5
@halfflat
SIMD min_align
51f36cc 
* Define, use, and propagate minimum alignment expectations for SIMD-based
mechansism.
* Update fvm_lowered unit test with new mechanism field access wrappers.
@halfflat
Use alignment and SIMD width correctly in multicore.
8f7c758 
* DTRT in multicore shared_state re: mechanism alignment and partition
width.
* Fix for uncalled procedure warning in SIMD generated code.
@halfflat
GPU fix ups WIP
00f5fa3
@halfflat
Fix GPU apply_events; other bugs remain
ddcef2c
@halfflat
GPU-side now passing unit tests.
d78e488
@thorstenhater
Add header.
b8a2dfa
@thorstenhater
Add @halfflat s contributions.
f50bf3d
@thorstenhater
Copy link
Contributor Author

bors try

@bors
Copy link

bors bot commented Jul 21, 2021

try

Merge conflict.

@thorstenhater
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jul 21, 2021
@bors
Try #1452:
c5e7c9c
@bors
Copy link

bors bot commented Jul 21, 2021

try

Build failed:

@thorstenhater
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jul 21, 2021
@bors
Try #1452:
bb824cc
@bors
Copy link

bors bot commented Jul 21, 2021

try

Build failed:

@halfflat
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Jul 28, 2021
@bors
Try #1452:
2c3cb26
@bors
Copy link

bors bot commented Jul 28, 2021

try

Build failed:

@halfflat
Copy link
Contributor

I'm going to merge despite the CSCS gitlab CI failure, which appears due to a problem with the way that git submodules are or are not updated in that process — manually building on the Fujitsu node demonstrates that unit tests compile and pass. There's a chance that once the new submodules are on master, it will fix itself; otherwise, we can address that as a separate issue.

@halfflat halfflat merged commit ff12bb8 into arbor-sim:master Jul 29, 2021
@thorstenhater thorstenhater deleted the feature/mech-abi-final branch August 1, 2022 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schmitts schmitts schmitts left review comments

@halfflat halfflat halfflat requested changes

@bcumming bcumming Awaiting requested review from bcumming

@noraabiakar noraabiakar Awaiting requested review from noraabiakar

Assignees

@halfflat halfflat

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reversal Potential Mechanisms are not executed where advertised Add Mechanism ABI Mechanism ABI
4 participants
@thorstenhater @bcumming @halfflat @schmitts