Add new benchmark: Arepo #328
Conversation
Hi Thomas, It is fine. I made the changes |
| with open('Makefile.systype', 'w') as f: | ||
| f.write('SYSTYPE="Darwin"\n') | ||
|
|
There was a problem hiding this comment.
Is this supposed to be always set to Darwin, or only on macOS?
There was a problem hiding this comment.
I am not quite sure but here is two way to do that:
The first thing after obtaining a local copy of the code is to set a SYSTYPE variable, either by typing:
export SYSYTPE="MySystype"
(assuming you run the bash-shell) or by copying the Template-Makefile.systype file:
cp Template-Makefile.systype Makefile.systype
and uncommenting the intended SYSTYPE variable in Makefile.systype.
There was a problem hiding this comment.
I think the question here was whether always setting this to Darwin is correct. On my Ubuntu laptop I need to manually change it to Ubuntu to get the include paths right, which is not ideal.
I would have assumed that you could just leave SYSTYPE undefined and spack would provide the right paths to the dependencies, but this does not seem to be the case. I didn't really dig deeper into the build system to find out what it's doing.
There was a problem hiding this comment.
This is definitely strange: I'd expect it to fail on a non-mac, which is what Tuomas is seeing, but it didn't fail for me on cosma. 🤔
There was a problem hiding this comment.
Perhaps on cosma when you have the right modules loaded you don't need to set include paths in the makefile and the SYSTYPE doesn't matter. I would like the build system to work without hard-coded paths, that would make life a lot easier.
There was a problem hiding this comment.
I agree with @tkoskela, it worth to escape hard-coded paths. They could cause mess for general perspective.
- Fix variant name - independent of cosma - Fix OS name in spack environment for cosma for new OS after upgrade - Add some new packages and compilers to spack environment for cosma post-upgrade (they may or may not be needed for AREPO...)
| variant('fftw', default=False, description='FFTW support') | ||
| variant('hdf5', default=True, description='HDF5 support') | ||
| variant('hwloc', default=False, description='HWLOC support') |
There was a problem hiding this comment.
How do these work? I don't see them being used to modify the build anywhere.
There was a problem hiding this comment.
See just below:
depends_on('fftw', when='+fftw')
It worked for me, default and variants.
There was a problem hiding this comment.
Yeah, but presumably this information should get passed to make somehow to build it without fftw
There was a problem hiding this comment.
Oh... Do we even want to build without fftw? The (very divergent) branch I'm looking at needs it by default. (I'll add a variant for that in another PR, once we merge this)
There was a problem hiding this comment.
Well, why have a variant for fftw if it's always required. Also default is False so by default FFTW support is off (but it doesn't actually do anything, other than remove fftw from the dependencies, which I imagine can cause issues)
There was a problem hiding this comment.
Unless I'm misunderstanding what 'FFTW Support' means here?
| with open('Makefile.systype', 'w') as f: | ||
| f.write('SYSTYPE="Darwin"\n') | ||
|
|
There was a problem hiding this comment.
I think the question here was whether always setting this to Darwin is correct. On my Ubuntu laptop I need to manually change it to Ubuntu to get the include paths right, which is not ideal.
I would have assumed that you could just leave SYSTYPE undefined and spack would provide the right paths to the dependencies, but this does not seem to be the case. I didn't really dig deeper into the build system to find out what it's doing.
This PR address the enhancements in issue #327 .