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

Include model name in DLL name on Windows #476

Merged
merged 4 commits into from
Nov 9, 2021
Merged

Conversation

neworderofjamie
Copy link
Contributor

Due to the inflexibility of Windows DLLfinding, runner_Release.dll (or runner_Debug.dll) is created in the same directory as the script (rather than in the ***_CODE sub-directory with everything else). Previously this was only a minor annoyance but, now the caching system (#430) means that models don't always get rebuilt, it can lead to an issue where, if you have multiple models in one directory, a DLL for the wrong model will get loaded.

This PR solves this by adding a flag which includes the model name in the DLL name (for backward compatibility, for now, this is not the default). In PyGeNN, as we're in control of both building and loading the DLL, this is now the default.

Copy link
Member

@tnowotny tnowotny 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. What's the reasoning behind not including the name by default outside of pygenn? Simply backward compatibility?

@neworderofjamie
Copy link
Contributor Author

If you were using GeNN in 'classic' C++ mode with this enabled, all your existing projects would be left pointing at libraries with the old style names which would no longer get updated. Will make this the default behaviour next major version.

@tnowotny tnowotny self-requested a review November 9, 2021 09:55
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #476 (594f6ab) into master (9d43b7c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #476   +/-   ##
=======================================
  Coverage   87.52%   87.52%           
=======================================
  Files          78       78           
  Lines       17278    17278           
=======================================
  Hits        15122    15122           
  Misses       2156     2156           
Impacted Files Coverage Δ
include/genn/genn/code_generator/backendBase.h 91.52% <ø> (ø)
src/genn/generator/generator.cc 90.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d43b7c...594f6ab. Read the comment docs.

@neworderofjamie neworderofjamie merged commit 2a09d50 into master Nov 9, 2021
@neworderofjamie neworderofjamie deleted the dll_name branch November 9, 2021 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants