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

Add spack package #486

Closed
wants to merge 5 commits into from
Closed

Add spack package #486

wants to merge 5 commits into from

Conversation

muffgaga
Copy link

@muffgaga muffgaga commented Nov 26, 2021 •
edited

@neworderofjamie That's what I used to get GeNN installed via spack. We probably need some more testing… should I just try to run the run_tests.sh? I tried, and there's an additional dependency on lcov and gtest that I need to add it seems… but also some other include-path(maybe?)-related fails. And another problem is that the test takes quite long (compared to normal install tests)… and it's not reporting errors via a non-zero exit code ;).

4176 model build complete
4177 make -C var_init_egp_CODE
4178 make[1]: Entering directory '/fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/features/var_init_egp/var_init_egp_CODE'
4179 make[1]: Leaving directory '/fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/features/var_init_egp/var_init_egp_CODE'
4180 g++ -std=c++11 -Wall -Wpedantic -Wextra -I  -isystem /include test.cc /src/gtest-all.cc /src/gtest_main.cc -o test -Lvar_init_egp_CODE -pthread -lrunner -Wl,-rpath var_init_egp_CODE
4181 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4182 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4183 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4184 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/unit /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4185 rm -f /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/obj_coverage/tests/unit/*.o /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/obj_coverage/tests/unit/*.d /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/unit/test_coverage
4186 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4187 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/spineml/simulator /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4188 rm -f /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/spineml/simulator/test *.d *.gcno
4189 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4190 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/spineml/generator /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4191 rm -f /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests/spineml/generator/test *.d *.gcno
4192 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4193 /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/tests
4194 make: *** No rule to make target 'clean'.  Stop.
4195 Makefile:1: *** missing separator.  Stop.
4196 cat: /fasthome/mueller/code/spack_chisel/opt/spack/linux-debian11-sandybridge/gcc-11.2.0/genn-4.6.0-sw6b6wqta7xkwljtgsar2ovfyqtuluva/version.txt: No such file or directory
4197 code_generator/generateSynapseUpdate.cc: In function 'void {anonymous}::applySynapseSubstitutions(CodeGenerator::CodeStream&, std::string, const string&, const CodeGenerator::SynapseGroupMergedBase&, const CodeGenerator::Substitutions&, const CodeGenerator::ModelSpecMerged&, bool)':
4198 code_generator/generateSynapseUpdate.cc:22:85: warning: unused parameter 'errorContext' [-Wunused-parameter]
4199    22 | void applySynapseSubstitutions(CodeStream &os, std::string code, const std::string &errorContext,
4200       |                                                                  ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
4201 g++: error: /include: No such file or directory
4202 g++: error: /src/gtest-all.cc: No such file or directory
4203 g++: error: /src/gtest_main.cc: No such file or directory
4204 make: *** [Makefile:8: test] Error 1

@healther Maybe you can also have a short look and provide some comments? I also plan to add this to upstream spack.

@muffgaga muffgaga mentioned this pull request Nov 26, 2021
Open
@neworderofjamie
Copy link
Contributor

Ahh yeah, run_tests.sh is mostly just run via Jenkins so you need to set GTEST_DIR to point to a google test install. Running it with no options should just run the tests on CPU which shouldn't take too long.

@muffgaga muffgaga force-pushed the add_spack_package branch 2 times, most recently from 4b3ac3a to 3282dbb Compare November 26, 2021 14:04
@muffgaga
Copy link
Author

muffgaga commented Nov 26, 2021 •
edited

Ahh yeah, run_tests.sh is mostly just run via Jenkins so you need to set GTEST_DIR to point to a google test install. Running it with no options should just run the tests on CPU which shouldn't take too long.

Hmm, spack's googletest package installs headers and builds libraries → should I modify the Makefiles to just link against libgtest.so instead of trying to build from source? (AFAIR we transitioned to this mode for the BrainScaleS software stack some years ago when major distributions switched to this no-src-but-lib-only deployment of gtest.)
(This can be done via spack-package-specific patches that get applied during the package build.)

@neworderofjamie
Copy link
Contributor

neworderofjamie commented Nov 26, 2021 •
edited

Ahh damn, I had no idea google test had done that - when we first started using it I'm sure there recommendation was just to build and link the C++ source. I'd need to modify the Jenkinsfile to checkout master of google test and build it with cmake rather than just extracting the release tar ball which sounds like a task for another day tbh. I'll add issues to do that and make run_tests.sh return a sensible status code

@muffgaga
Copy link
Author

Ahh damn, I had no idea google test had done that - when we first started using it I'm sure there recommendation was just to build and link the C++ source. I'd need to modify the Jenkinsfile to checkout master of google test and build it with cmake rather than just extracting the release tar ball which sounds like a task for another day tbh. I'll add issues to do that and make run_tests.sh return a sensible status code

Sounds good, thx! (And I implemented some "patching" for the meantime… :))

@muffgaga muffgaga force-pushed the add_spack_package branch 3 times, most recently from dbd33ef to 848de21 Compare November 26, 2021 17:10
@neworderofjamie neworderofjamie added this to the GeNN 4.7.0 milestone Nov 26, 2021
@muffgaga muffgaga force-pushed the add_spack_package branch 2 times, most recently from 38d72c4 to 1c8da78 Compare November 29, 2021 09:38
@muffgaga
Copy link
Author

muffgaga commented Nov 29, 2021 •
edited

After patching some more gtest things → run_tests.sh seems to finish almost correctly (there are some warnings, but no hard errors)…?

genn-4.6.0-q6k5md6-test-out.txt

@neworderofjamie
Copy link
Contributor

the errorContext one is a longstanding reminder for me to fix something but the other one is a good spot from new GCC. If you could add an & to line 229 of src/spineml/simulator/inputValue.cc as part of that PR, that would be wonderful.

@muffgaga
Copy link
Author

[add &]

Sure :)…

tags: linux enhancement

btw. spack also works on Mac and probably most other Unixes… :)
…and all Windows-based development happens in WSL2 nowadays :p?

@neworderofjamie
Copy link
Contributor

Thanks! I don't think CUDA passthrough has made it to any but the most bleeding edge Windows update streams so we have plenty of native Windows users (myself included)

@muffgaga
Copy link
Author

muffgaga commented Nov 30, 2021 •
edited

Hmm, no windows support in spack yet… cf. spack/spack#9323

BTW. I fixed another issue: I believe tests/features/batch_decode_matrix_conn_gen/Makefile was supposed to be a symlink. Commit added on top…
Maybe a bash -e for the shell script would be nice (to fail early… -o pipefail could be also helpful).

@muffgaga
Copy link
Author

And one more missing Makefile added…

@neworderofjamie
Copy link
Contributor

There are still a few of those Makefiles which should be symlinks knocking around - thank you!

@muffgaga
Copy link
Author

Is there anything left that I should modify/fix/add?
My manual testing looks ok (which is basically some manual runs of spack install genn; spack test run genn in the variants genn+python~cuda, genn~python etc…).

@neworderofjamie
Copy link
Contributor

That all looks good to me - when we update to building gtest as a library, a lot of the hacks will disappear. One small question - do the depends_on lines explicitly pull in that version? For the Python stuff that seems like a bad idea as you'd be left with ancient versions of numpy etc which will conflict with other packages - numpy is required for PyGeNN setup which is awkward but the rest could just be left to setup tools to install.

@neworderofjamie
Copy link
Contributor

Shit! Your fixing of symlinks has uncovered an AMD-specific OpenCL issue 🤮

@muffgaga
Copy link
Author

muffgaga commented Dec 2, 2021 •
edited

[…] One small question - do the depends_on lines explicitly pull in that version? […]

No, it's all ranged versions (cf. https://spack.readthedocs.io/en/latest/basic_usage.html#version-specifier for details) → anything above (and including) Python 3.8.0, numpy >= 1.17.0 and >= 1.0.0 for importlib metadata thingy… the other dependencies are unspecified in terms of versions.
Typically spack takes the LATEST version known to spack that fits together with all the other dependencies. (There's also a preferred=True kwarg that can be added to specific version in the package file of packages to change this behavior; and users can specify constraints on dependency via ^dependency@something syntax.)

[…] the rest could just be left to setup tools to install.[…]

But that would avoid a spack-tracked software environment, i.e. in the general case this will lead to conflicting package specifications ⇒ that's the main reason for using spack → tracking ALL dependencies in a DAG and solving the requirements to arrive at a valid "concretization" of this graph.
(There is a nice talk about how spack solves this problem…)

@neworderofjamie neworderofjamie linked an issue Dec 9, 2021 that may be closed by this pull request
Request for comment: Spack package for GeNN #485
Open
@neworderofjamie
Copy link
Contributor

neworderofjamie commented Dec 9, 2021 •
edited

Ok, it's not clear to me why Jenkins is still saying this failed as it did not.

neworderofjamie
neworderofjamie reviewed Dec 9, 2021
View reviewed changes
spack/include_path.patch

// CLI11 includes
-#include "../../include/genn/third_party/CLI11.hpp"
+#include "genn/third_party/CLI11.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly does this patch aim to fix?

Copy link
Contributor

Choose a reason for hiding this comment

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

@muffgaga - am just trying to tidy up before releasing 4.7.0 - I'd like to get this merged but I still don't quite understand why this is being patched

@neworderofjamie neworderofjamie removed this from the GeNN 4.7.0 milestone Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neworderofjamie neworderofjamie neworderofjamie left review comments

Assignees
No one assigned
Labels
enhancement linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request for comment: Spack package for GeNN
2 participants
@muffgaga @neworderofjamie