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

Handling TABLE statements when CoreNEURON is enabled with GPU support (Part I) #1505

Open
pramodk opened this issue Oct 17, 2021 · 4 comments

Comments

@pramodk
Copy link
Member

pramodk commented Oct 17, 2021

Overview of the feature

Currently CoreNEURON+MOD2C doesn't support TABLE statement on the GPU. We plan to implement this support but it's pending for quite some time due to other ongoing tasks.

One of the reason we didn't prioritise this feature is that user can easily comment out TABLE statement and run the model. This is also what we do for hh.mod when CoreNEURON is enabled via CMake option -DNRN_ENABLE_CORENEURON=ON.

Today, if user does nrnivmodl -coreneuron <mod> with GPU installation then compilation on coreneuron side fails. We can ask user to comment out TABLE statements but I wonder what if we _disable statements automatically if mod files are being compiled with coreneuron+gpu support ?

For example:

  • Assume user has installed neuron+coreneuron with GPU support
  • If user invoke nrnivmodl <mod-dir> then everything is compiled as it is i.e. including TABLE statements
  • If user invoke nrnivmodl -coreneuron <mod-dir> then 1) nocmodl parse & ignore TABLE statements in the mod file 2) mod2c parse and ignore TABLE statements.

Note that ignoring statements on both translators is important in order to match the data structure sizes (data and pdata arrays). This could be achieved via some extra CLI flag to nocmodl and mod2c.

Foreseeable Impact

This is not ideal solution but instead of asking user to comment the MOD files I think it would make sense to do it ourselves until TABLE statements are supported. And, as we are doing this only when user ask to enable coreneuron on GPU (via -coreneuron option go nrnivmodl, I assume this doesn't affect "normal usage".

@nrnhines
Copy link
Member

Yes, "not ideal" in the unlikely event the user experiences confusion if noticeing slightly different results depending on nrnivmodl and nrnivmodl -coreneuron and is not aware of the reason. However, I'm willing to accept as a temporary solution. I assume there will be a warning message generated by the translator (although not likely to be seen in the midst of all the other output generated).

@pramodk
Copy link
Member Author

pramodk commented Oct 19, 2021

@nrnhines : Another such pattern / construct is a GLOBAL variables : if there is a GLOBAL variable in a THREADSAFE mod file then NEURON promote that variable to a thread variable. But this promotion is not sufficient on CoreNEURON side due to SIMD execution. We need to promote such variable to RANGE. What if we do this promotion consistently across neuron and coreneuron? This often causes bug and undefined behaviour due to a race condition. (as I just pointed out in other slack discussion)

@olupton
Copy link
Collaborator

olupton commented Aug 24, 2022

It seems that TABLE statements work with CoreNEURON + NMODL + GPU with BlueBrain/nmodl#904.

This is largely accidental: that PR changes the data structure so that the TABLE data is included directly in a structure of mechanism-global variables that is copied to the GPU, whereas previously there was an extra layer of pointer indirection to reach the TABLE data that was not (painstakingly) navigated when copying data to the GPU.

CoreNEURON + MOD2C + GPU presumably still does not work when TABLE statements are enabled.

Of course, none of this is to say that using TABLE statements on GPU is actually a good idea...

@pramodk
Copy link
Member Author

pramodk commented May 8, 2024

It's worth updating this issue in 2024!

As far as I understand, here are the steps that we need to take:

Step 1: to verify if NMODL is correctly handling TABLE statements on GPU:

  • If TABLE statements are indeed supported then we shouldn't need to toggle those statements in the nrn's hh.mod file, see the CMake code here. The logic about TABLE_VAR_TOGGLE_COMMAND should be removed.
  • We should also comment out coreneuron side TABLE statement from here
  • Now build the NEURON on GPU with tests enabled and see if all tests pass.
    • Important Note: if we see some tests have reference results generated with the TABLE statement commented then we need to change those reference results!
    • Once we change such reference results then all tests should be green.

We have multiple tests with online mode (e.g. ringtests) then they should pass on GPU.

Step 2: Cross-check performance aspects with the TABLE statements (on CPU + GPU)

Todo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants