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

Make hh.mod configure-able at compile-time #3294

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ virtualenv
docs/_build
docs/_generated
.vscode
src/nrnoc/hh.mod
24 changes: 0 additions & 24 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -789,30 +789,6 @@ add_custom_target(
COMMENT "Format only files modified with respect to master branch."
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR})

# =============================================================================
# ~~~
# Update hh.mod for CoreNEURON compatibility
# - Replace GLOBAL variable by RANGE
# - Comment out TABLE
# ~~~
# =============================================================================
if(NRN_ENABLE_CORENEURON OR NRN_ENABLE_MOD_COMPATIBILITY)
set(GLOBAL_VAR_TOGGLE_COMMAND "'s/ GLOBAL minf/ RANGE minf/'")
else()
set(GLOBAL_VAR_TOGGLE_COMMAND "'s/ RANGE minf/ GLOBAL minf/'")
endif()
separate_arguments(GLOBAL_VAR_TOGGLE_COMMAND UNIX_COMMAND "${GLOBAL_VAR_TOGGLE_COMMAND}")
add_custom_target(
hh_update
COMMAND sed ${GLOBAL_VAR_TOGGLE_COMMAND} ${CMAKE_SOURCE_DIR}/src/nrnoc/hh.mod >
${CMAKE_BINARY_DIR}/hh.mod.1
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_BINARY_DIR}/hh.mod.1
${CMAKE_SOURCE_DIR}/src/nrnoc/hh.mod
COMMAND ${CMAKE_COMMAND} -E remove ${CMAKE_BINARY_DIR}/hh.mod.1
COMMENT "Update hh.mod for CoreNEURON compatibility"
VERBATIM)
add_dependencies(nrniv_lib hh_update)

# =============================================================================
# Generate help_data.dat
# =============================================================================
Expand Down
5 changes: 5 additions & 0 deletions cmake/MacroHelper.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ macro(nocmodl_mod_to_cpp modfile_basename)
if(CMAKE_VERSION VERSION_LESS "3.17")
set(REMOVE_CMAKE_COMMAND "remove")
endif()
set(MODFILE_CONFIG_FILE ${PROJECT_SOURCE_DIR}/${modfile_basename}.mod.in)
if(EXISTS ${MODFILE_CONFIG_FILE})
configure_file(${MODFILE_CONFIG_FILE} ${PROJECT_SOURCE_DIR}/${modfile_basename}.mod @ONLY)
endif()

get_filename_component(modfile_output_dir ${PROJECT_SOURCE_DIR}/${modfile_basename}.mod DIRECTORY)
add_custom_command(
OUTPUT ${PROJECT_BINARY_DIR}/${modfile_basename}.cpp
Expand Down
11 changes: 11 additions & 0 deletions src/nrniv/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ endif()
# Translate all MOD files to C and mark them generated
# =============================================================================
foreach(modfile ${NRN_MODFILE_BASE_NAMES})
# =============================================================================
# ~~~
# Set the correct supported keyword for CoreNEURON compatibility
# - Replace GLOBAL variable by RANGE
# ~~~
# =============================================================================
if(NRN_ENABLE_CORENEURON OR NRN_ENABLE_MOD_COMPATIBILITY)
set(NRN_SUPPORTED_KEYWORD "RANGE")
else()
set(NRN_SUPPORTED_KEYWORD "GLOBAL")
endif()
nocmodl_mod_to_cpp(${modfile})
list(APPEND NRN_MODFILE_CPP ${PROJECT_BINARY_DIR}/${modfile}.cpp)
endforeach()
Expand Down
4 changes: 2 additions & 2 deletions src/nrnoc/hh.mod → src/nrnoc/hh.mod.in
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ NEURON {
USEION k READ ek WRITE ik REPRESENTS CHEBI:29103
NONSPECIFIC_CURRENT il
RANGE gnabar, gkbar, gl, el, gna, gk
: `GLOBAL minf` will be replaced with `RANGE minf` if CoreNEURON enabled
GLOBAL minf, hinf, ninf, mtau, htau, ntau
: set at compile-time to `GLOBAL` if using NEURON, `RANGE` if using CoreNEURON
@NRN_SUPPORTED_KEYWORD@ minf, hinf, ninf, mtau, htau, ntau
Copy link
Member

Choose a reason for hiding this comment

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

@JCGoran : technically, this is correct but I just want to ask @nrnhines if there is a "preference" to have hh.mod in usable format. E.g. if someone wants to copy that directly and compile it. Can't think of use case but just want to ask it in advance.

Copy link
Member

Choose a reason for hiding this comment

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

"preference"

I don't know. This usage (exploiting GLOBAL for efficiency to effectively implement a multiple value return from a function) predates threads. And then, with a lot of internal shenanigans, promoting them to thread variables to avoid races. And then (for GPU) just replacing with RANGE when there are many many threads. The whole thing contradicts the desire for description instead of programming and we have been working around the problem ever since.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pramodk hence the "one way to fix" part 😅 Another option would be to move the mod files from src/nrnoc to a subdir, like src/nrnoc/modfiles, which has 2 additional subdirs, like neuron and coreneuron, and depending on whether CoreNEURON is used or not, we use the mod files from the corresponding subdir. The upside of that is that it avoids configure_file magic (and wouldn't require yet another cmake variable, which are already hard to keep track of as it is), so all the mod files are still usable on their own. The backend-agnostic mod files could reside in modfiles/neuron, with their modfiles/coreneuron counterparts just being symlinks to those, while hh.mod would need to have 2 versions, one for NEURON, and one for CoreNEURON.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've made an alternative implementation in #3295 if that one would be more appropriate.

THREADSAFE : assigned GLOBALs will be per thread
}

Expand Down
Loading