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

New Package: lcals #4792

Merged
merged 9 commits into from
Jul 27, 2017
Merged

New Package: lcals #4792

merged 9 commits into from
Jul 27, 2017

Conversation

pkondamudi
Copy link
Contributor

No description provided.

@pkondamudi pkondamudi changed the title New Pacakge: lcals New Package: lcals Jul 17, 2017
targets = []

if self.compiler.name == 'gcc':
targets.append('LCALS_ARCH={}'.format('x86_sse_gnu'))
Copy link
Member

Choose a reason for hiding this comment

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

This won't work in Python 2.6. You need to use {0} instead of {}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed this.

targets.append('CXX={}'.format('icc'))
if self.compiler.name == 'xl':
targets.append('LCALS_ARCH={}'.format('bgp_xlc9'))
targets.append('CXX={}'.format('mpixlcxx'))
Copy link
Member

Choose a reason for hiding this comment

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

This executable does not come from the %xl compiler, it is an MPI wrapper presumably built using %xl.

Copy link
Contributor Author

@pkondamudi pkondamudi Jul 17, 2017

Choose a reason for hiding this comment

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

From Line 56 and 57, I'm assuming that mpixlcxx is installed on the machine and gets invoked through Makefile.

Copy link
Member

Choose a reason for hiding this comment

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

That's not a very good assumption. If you want to build it with an MPI compiler wrapper, that's fine, but you need to build it with an MPI dependency. You could just as easily build it with an MPI compiler wrapper using the Intel compiler, or without MPI while using Xl.

targets.append('LCALS_ARCH={}'.format('x86_sse_gnu'))
targets.append('CXX={}'.format('g++'))
if self.compiler.name == 'intel':
targets.append('LCALS_ARCH={}'.format('MIC'))
Copy link
Member

Choose a reason for hiding this comment

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

The architecture and the compiler have nothing in common. You can build non-MIC with Intel and you can use compilers other than %xl on blue-gene.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm setting targets.append('LCALS_ARCH={}'.format('MIC')) to make sure the other logics in the Makefile executes correctly when using this compiler.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but what if you need to build it with MIC support using a different compiler? And what if you don't need to build it with MIC support but you want to use Intel? I don't see how these things are related.

variant(
'xlcn',
default=9,
description='Version number for XLC compiler 9 or 12.',
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to get the version number of the compiler through self.compiler.version.

Copy link
Member

Choose a reason for hiding this comment

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

Also, what happens if someone needs to build with XLC 10? Another reason not to use LCALS_ARCH.

variant(
'microarch',
default=1,
description='Micro arch: 1 - SSE, 2 - AVX.',
Copy link
Member

Choose a reason for hiding this comment

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

Don't use a numeric variant, use values=('sse', 'avx')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

if xlcn == '9':
cxx = 'mpixlcxx'
elif xlcn == '12':
cxx = 'mpixlcxx_r'
Copy link
Member

Choose a reason for hiding this comment

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

Once again, these are not compilers, they are compiler wrappers. You're welcome to build this package with MPI support, but you need to add an optional MPI dependency, and only use the compiler wrappers when +mpi. It's just as easy to build with mpiicc as it is to build with xlc.

Copy link
Contributor Author

@pkondamudi pkondamudi Jul 18, 2017

Choose a reason for hiding this comment

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

I'm not the author of the actually Makefile. So, instead of making a decision on using mpicc, I choose to assume that mpi wrappers are installed on the machine there by gets invoked from Makefile. The reason why I'm setting CXX in Spack script because Makefile is insisting the user to have those in a specific location with hardcoded values no filesystem .

Copy link
Member

Choose a reason for hiding this comment

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

You can use any CXX you want to. You don't need to use what the Makefile does.

Copy link
Member

Choose a reason for hiding this comment

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

'CXX={0}'.format(spack_cxx)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@adamjstewart
Copy link
Member

I just took a closer look at the Makefile. If you look, you'll see that LCALS_ARCH is only used to set CXX, CXX_COMPILE, CXXFLAGS, etc. I think your best bet is to completely ignore LCALS_ARCH and set these variables manually based on the compiler. CXX will always be spack_cxx regardless of which compiler you are using. As for the other variables, just choose them based on what the Makefile uses.

arch = platform.machine()

if microarch != 'sse' and microarch != 'avx' and microarch != 'MIC':
raise InstallError('Invalid choice:-{0}-. Micro arch: 1 - SSE, 2 - AVX.'.format(microarch))
Copy link
Member

Choose a reason for hiding this comment

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

You don't need this anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they are making change in Makefile by adding XLC10, I assume that they need to add it in Spack as well and just trying to make current Makefile and Spack scripts to be compatible.

else:
raise InstallError('Fatal Error: unknown construct.')

targets.append('LCALS_ARCH={0}'.format(arch))
Copy link
Member

Choose a reason for hiding this comment

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

I still wouldn't bother setting LCALS_ARCH, but it's up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

want Makefile to take care of setting flags else Spack script could become complicated.

Copy link
Member

Choose a reason for hiding this comment

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

The Spack script is already complicated 😆 Seriously, if you use the LCALS_ARCH thing, it isn't possible to use the XL compiler unless you're using version 9 or 12.

variant(
'xlcn',
default=9,
description='Version number for XLC compiler 9 or 12.',
Copy link
Member

Choose a reason for hiding this comment

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

Also, what happens if someone needs to build with XLC 10? Another reason not to use LCALS_ARCH.

try:
return isinstance(int(x), numbers.Integral) and not isinstance(x, bool)
except ValueError:
return False
Copy link
Member

Choose a reason for hiding this comment

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

You don't need this function anymore.

cxxflags += '-DLCALS_PLATFORM_X86_SSE -DLCALS_COMPILER_GNU '
cxx_compile += '-Ofast -msse4.1 -finline-functions'
' -finline-limit=10000 -std=c++11 '
print('Im here!!!!')
Copy link
Member

Choose a reason for hiding this comment

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

I think you forgot to remove this print statement.

' -std=c++0x'
cxxflags += self.compiler.openmp_flag
elif self.compiler.name == 'xl' and arch == 'bgp':
if self.compiler.version == 9 and arch == 'bgp':
Copy link
Member

Choose a reason for hiding this comment

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

Did you check that this actually works? You might have to do Version('9') instead, I'm not sure.

elif self.compiler.name == 'xl' and arch == 'bgp':
if self.compiler.version == 9 and arch == 'bgp':
cxxflags += '-DLCALS_PLATFORM_BGP -DLCALS_COMPILER_XLC9'
' -I/usr/gapps/bdiv/sles_10_ppc64/opt/platform/include '
Copy link
Member

Choose a reason for hiding this comment

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

This directory won't exist on most computers.

' -I/usr/gapps/bdiv/sles_10_ppc64/opt/platform/include '
cxx_compile += 'O3 -qarch=450d -qtune=450 -qalias=allp -qhot'
' -qsmp=omp '
ldpath += '-L/usr/local/tools/lib'
Copy link
Member

Choose a reason for hiding this comment

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

You probably don't want to pick up any system libraries with this.

@adamjstewart
Copy link
Member

I think your last commit undid some of your changes?

@adamjstewart adamjstewart merged commit ce601a9 into spack:develop Jul 27, 2017
mamelara added a commit to NERSC/spack that referenced this pull request Aug 1, 2017
* satsuma2: new package (spack#4838)

* salmon: new package (spack#4833)

* New Package: C-Ares (spack#4840)

Adds the c-ares library, a C library for asynchronous DNS requests.

Required for the google gRPC library.

* sickle: new package (spack#4851)

* seqprep: new package (spack#4850)

* smalt: new package (spack#4853)

* singularity: new package (spack#4852)

* lmdb: Update to 0.9.21 (spack#4830)

Convert to MakefilePackage and add pkg-config file.

* added new pruners-ninja version (spack#4859)

* sortmerna: new package (spack#4866)

* revbayes: trying this again (spack#4861)

* new package: miniGMG (spack#4849)

* new package: miniGMG

* changed based on comments

* removed cuda version

* sparta: new package (spack#4867)

* sparta: new package

* fixing homepage

* Savanna (spack#4856)

Installing the stable version 0.5 through the checksummed tar.gz does not fetch the git submodule in the package.
The submodule appears as an empty directory.

Thus, clone the commit tagged as v0.5 using git to get around this issue

* savanna: modified adios dependency spec
* Replaced adios+staging with adios+flexpath+dataspaces
* savanna: Enabling fortran support in adios by default
* savanna: reverting to variant 'staging' for enabling all staging transports

* Make testing spack commands simpler (spack#4868)

Adds SpackCommand class allowing Spack commands to be easily in Python

Example usage:

    from spack.main import SpackCommand
    info = SpackCommand('info')
    out, err = info('mpich')
    print(info.returncode)

This allows easier testing of Spack commands.

Also:
* Simplify command tests
* Simplify mocking in command tests.
* Simplify module command test
* Simplify python command test
* Simplify uninstall command test
* Simplify url command test
* SpackCommand uses more compatible output redirection

* gBenchmark: Development Package (spack#4847)

* gBenchmark: Development Package

Add the development version (master branch) of `gBenchmark`

* gBenchmark: Remove Duplicate

Remove duplicate `gbenchmark` library
and keep its patch to remove the shipped
-Werror

* Perl - allow package activation without PERL5LIB variable (spack#4540)

* perl: prepend default perl @inc path to support package activation

* perl: remove stray comma from list of configure arguments

* perl: final comma in configure arguments makes adding arguments safer

This reverts commit fdc10cd.

* perl: add comment about modified @inc (thanks to George Hartzell)

* perl: use self.prefix.lib and self.prefix.bin for clarity

* perl: convert tabs added by editor to spaces for flake8

* perl: use new path syntax: prefix.lib.perl5

* perl: avoid line break before binary operator

* perl: use compact spack syntax for perl executable

* fix sphinx dependencies, add v1.6.3 (spack#4870)

* Add cuda variant for mvapich2. (spack#4800)

* Add cuda variant for mvapich2.

* Disable cuda for mvapich2 by default.

* Add a py-theano version from git repo (spack#4871)

* Change Version formatting properties and functions to return Version objects (spack#4834)

* Change version.up_to() to return Version() object
* Add unit tests for Version.up_to()
* Fix packages that expected up_to() to return a string
* Ensure that up_to() preserves separator characters
* Use version indexing instead of up_to
* Make all Version formatting properties return Version objects
* Update docs
* Tests need to test string representation

* stringtie: new package (spack#4878)

* added new version of cdo (spack#4877)

* subread: new package (spack#4882)

* structure: new package (spack#4879)

* structure: new pacakge

* fixing package structure (not a pun)

* swarm: new package (spack#4885)

* added new version of jdk 8u141-b15 (spack#4876)

* stacks: new package (spack#4875)

* fix GobjectIntrospection on Darwin (spack#4872)

* fix GobjectIntrospection on Darwin

* minor

* Rename the gpu variant to cuda, this is to be consistent with other (spack#4890)

packages.

* Update zstd version (spack#4873)

* Update zstd version

* Change order of versions

* Use MakefilePackage

* sumaclust: new package (spack#4884)

* sumaclust: new package

* tweaking url and make specs

* tabix: new package (spack#4886)

* tabix: new package

* fixed docs location

* cleaveland4: new package (spack#4894)

* cleaveland4: new package

* fixing return line in viennarna url_for_version

* fix config.guess patch for ppc64le (spack#4858)

* fix config.guess patch for ppc64le

* explicit patch for config.guess not required

* trimgalore: new package (spack#4899)

* transposome: new package (spack#4896)

* transdecoder: new package (spack#4895)

* transdecoder: new package

* fixed package structure

* fix callpath bug (spack#4659)

* fix callpath bug I found while testing env/cc

* fix hanging indent for flake

* tmux should not set PKG_CONFIG_PATH (spack#4901)

* fixes spack#967

* Version bump to 0.9.1

- Bugfixes for spack find
- 0.9.1 can read specs from current develop.

* Don't assume spack is in the path when building docs.

* Remove PKG_CONFIG_PATH from tmux configure

* Change tmux to AutotoolsPackage

* Correct link to libtinfo in tmux

* Add universal build_type variant to CMakePackage (spack#4797)

* Add universal build_type variant to CMakePackage
* Override build_type in some packages with different possible values
* Remove reference to no longer existent debug variant
* Update CBTF packages with new build_type variant
* Keep note on build size of LLVM

* shortstack: new package (spack#4905)

* added MPI dependency to Nekbone package (spack#4903)

* removed the tags as per comment in PR# 4749

* addressed above comments

* changed fortran compiler.

* added proxy application tags.

* added tags by removing them from description.

* addressed comments

* used join_path instead of path concat.

* removed the tags as per comment in PR# 4749

* addressed above comments

* changed fortran compiler.

* added proxy application tags.

* added tags by removing them from description.

* addressed comments

* used join_path instead of path concat.

* added tags.

* changes to use MPI as depedency.

* removed MPI as variant.

* changed pointer to filtered makenek file.

* flake 8 fix.

* Updated Namespace of BML Repository (spack#4910)

* Improve version detection for URLs with dynamic after version (spack#4902)

* snptest: new package (spack#4900)

* snptest: new package

* fixed version things

* fixed install phase

* openblas: add 0.2.20 (spack#4915)

* New Package: RSbench (spack#4752)

* New Package: RSbench

* minor change

* removed tags as per PR# 4749

* addressed comments and added gcc compiler.

* added proxy app tags to description.

* removed setting CC to pgicc through spec.

* removed compiler as depedency

* removed pgi variant.

* flake 8 fix.

* added mpi depedency with pgi compiler

* added  pgi compiler

* removed PGI compiler as depedency.

* added tags and addressed other code formattings.

* added tags and addressed other code formattings.

* addressed comments.

* Fix for Krell openspeedshop spack package bug.  New multi-value variant for GUI build. (spack#4880)

* Update the krell institute products to use the latest features of spack for building on cluster platforms.

* Address travis error messages and resubmit the pull request.

* Update the contents of openspeedshop package.py so it passes the flake8 tests.

* Fix flake8 error-whitespack issue in mrnet package.py file.

* Add updates based on spack reviewer feedback.

* More fixes based on comments from reviewers.  Switch using extend to using append, remove additional setting of PATH and LD_LIBRARY_PATH that should not be required due to RPATH.

* More review related changes.  Update MPIOption.append lines and take out xercesc references.

* Create a base options function for common openspeedshop base cmake options to reduce redundencies.

* Add libxml2+python depends on to get around issues with the libxml2 package file.

* Using boost over 1.60.0 causes compile errors.  This is a known boost bug. Also, dyninst-9.2.0 is set to be the vesrion of dyninst to use with OSS, as of now. The newer version fails to build.

* Fix bad syntax in specifying the boost version range.

* Update the version numbers for the krell institute components and tools: cbtf and openspeedshop.

* Do not build glib for qt3, it is not needed and causes build problems at this time anyway.

* A fix was added for setting LD_LIBRARY_PATH in the qt3 build, but if LD_LIBRARY_PATH is not set the qt build fails. So so check and set LD_LIBRARY_PATH if not set, update if it is set.

* Update the fix for qt3 build by setting LD_LIBRARY_PATH instead of checking for whether it is set or not per Adams comment that spack clears LD_LIBRARY_PATH.

* A fix was added for setting LD_LIBRARY_PATH in the qt3 build, but if LD_LIBRARY_PATH is not set the qt build fails. So so check and set LD_LIBRARY_PATH if not set, update if it is set.

* Trim comments to fit more concisely.

* Fix tabs versus spaces and swap if and else clause check from a negative to a positive check.

* Fix issues with the cbtf-argonavis build, update to use dyninst-9.3.2, fixes to openspeedshop package build.

* Fix issues with the cbtf-argonavis package.py files related to comments.

* Add changes for changing the krell packages from Package to CMakePackage.

* Add better changes for changing the krell packages from Package to CMakePackage.

* Add more modifications for changing the krell packages from Package to CMakePackage.

* Add additional modifications for changing the krell packages from Package to CMakePackage and fixing Travis erros

* Fix new travis errors.

* Fix new travis errors.

* Add more changes for PR 4765.

* Add more refinements to the conversion from Package to CMakePackage.

* Fix new travis errors.

* Add dependencies for MPI to be passed to cbtf-krell, so it can build the MPI collectors requested by the builder of openspeedshop.

* Remove extra unnecessary routine to adjust build arguments. Fix if-else clause issue.

* Fix more flake issues caused by last changes.

* Fix a bug where openspeedshop will not build when no mpi variants are specified.  Also switch to a multiple level variant for building the gui(s).  Use none, qt3, and qt4 as the variants with qt3 being the default.

* Add fix for spack issue spack#4843, where LTDL include files were not found.

* Add the build_type variant back into the openspeedshop package file.

* New Package: lcals (spack#4792)

* New Pacakge: lcals

* added logic for arch detection and compiler choice.

* fixes for comments.

* addressed comments.

* removed LCALS_ARCH and added flags though spack.

* addressed comments.

* flake 8 fix.

* reerted the changes along with comments.

* Added Proxy App tag (spack#4917)

* Added Proxy App tag

*  NO changes except proxy app tag

* bml: fix homepage (spack#4918)

* Adding QWT package. (spack#4911)

* Adding QWT package.

* Using builtin file filtering.

* Formatting.

* Fix for m4%clang (spack#4912)

* Fix for m4%clang

* Restricted condition to not subsitute rtlib on OSX

* Initial Spackage for qmd-progress library (spack#4924)

* Initial Spackage for qmd-progress library

PROGRESS is a library is focused on the development of general solvers
that are commonly used in quantum chemistry packages.

* Removed LA-CC from description to fix formatting

* Added Additional Formatting Requests

Added requested formatting changes and also ensured that graphlib and
mpi are disabled if not enabled

* gBenchmark: v1.2.0 (spack#4935)

Adds a the latest version of gBenchmark, release 1.2.0.

This is the first gBenchmark version with proper
[CMake config package installs](google/benchmark#363).
This is important for dependencies building against it, such as gRPC.

* zsh: add variant that skips tcsetpgrp test (spack#4923)

zsh's configure script fails if there's it tries to test for terminal
functionality if there's not a terminal (e.g. in a Jenkins build).

The configure script has a switch that asserts that tcsetpgrp works
and thereby avoids running that test.

This commit adds a variant that invokes that switch, defaulting to
True.

* Add latest version of apr (spack#4931)

* Add latest version of expat (spack#4930)

* Add missing dependencies to unixodbc (spack#4928)

* ZeroMQ: C++ Headers (cppzmq) (spack#4841)

Adds the cppzmq library, adding a C++ API to ZeroMQ (libzmq). In order to find the autotools-build libzmq, this requires the upcoming cppzmq release (or development branch).

* Add latest version of SCons (spack#4929)

* Add --color=[always|never|auto] argument; fix color when piping (spack#3013)

* Disable spec colorization when redirecting stdout and add command line flag to re-enable
* Add command line `--color` flag to control output colorization
* Add options to `llnl.util.tty.color` to allow color to be auto/always/never
* Add `Spec.cformat()` function to be used when `format()` should have auto-coloring

* Fix preference for X.Y version when mixed with X.Y.Z versions (spack#4922)

For packages which contain a mix of versions with formats X.Y and
X.Y.Z, if the user entered an X.Y version as a preference in
packages.yaml, Spack would get confused and favor any version A.B.Z
where X=A and Y=B. In the case where there is a mix of these version
types, this commit updates preferences so Spack will favor an exact
match.

* Clarify docs on using a hash in a spec (spack#4908)

* Fix xsdk build broken by petsc and trilinos (spack#4893)

* Fix xsdk build broken by petsc and trilinos

See spack#4891 for details

* Fix version conflict in trilinos package

Trilinos version 11 may conflict with superlu-dist.
The version "xsdk-0.2.0" was conflicting with superlu-dist,
even though it shouldn't.  I added a lower bound to the
comparison to fix this problem.

Thanks for the help @davydden!
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.

None yet

2 participants