Skip to content

Unified environment updates for Intel and netcdf-c #221

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

Merged
climbfuji merged 15 commits into from
Feb 15, 2023
Merged

Unified environment updates for Intel and netcdf-c #221

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8137622
Restrictions on newer package versions with Intel 18
climbfuji Feb 8, 2023
183612f
Merge branch 'jcsda_emc_spack_stack' of https://github.com/NOAA-EMC/s…
climbfuji Feb 9, 2023
06bbd97
libiconv@1.16 doesn't build with Intel@18
climbfuji Feb 9, 2023
1a3537b
Added version statement for netcdf-c version 4.9.1
srherbener Feb 9, 2023
f719860
Merge branch 'feature/netcdf-c-4.9.1' of https://github.com/srherbene…
climbfuji Feb 9, 2023
c738cd9
Merge branch 'jcsda_emc_spack_stack' into feature/netcdf-c-4.9.1
climbfuji Feb 9, 2023
0ca74ed
Merge branch 'feature/netcdf-c-4.9.1' of https://github.com/srherbene…
climbfuji Feb 9, 2023
79f2cb6
Fix conflict statement for libiconv
climbfuji Feb 8, 2023
f98228d
Fix CDO compile errors with Intel
climbfuji Feb 10, 2023
7754d38
Restrict googletest versions for Intel 18
climbfuji Feb 10, 2023
3001382
Add byterange variant for netcdf-c
climbfuji Feb 13, 2023
9b77f0e
Revert some Intel 18 changes
climbfuji Feb 13, 2023
27e61e5
Merge branch 'jcsda_emc_spack_stack' of https://github.com/NOAA-EMC/s…
climbfuji Feb 14, 2023
f1524e2
Lower optimization for nco with Intel18
climbfuji Feb 14, 2023
683f0c5
Fix netcdf-c, update nco with enable_or_disable, add missing packages…
climbfuji Feb 14, 2023
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
6 changes: 6 additions & 0 deletions var/spack/repos/builtin/packages/cdo/package.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ def yes_or_prefix(spec_name):
if self.spec.satisfies("@1.9:+hdf5^hdf5+mpi"):
flags["CPPFLAGS"].append("-DOMPI_SKIP_MPICXX -DMPICH_SKIP_MPICXX")

# Workaround compiler issues
# https://github.com/NOAA-EMC/spack-stack/issues/468
if self.spec.satisfies("%intel"):
flags["CFLAGS"].append("-O1")
flags["CXXFLAGS"].append("-O1")

config_args.extend(["{0}={1}".format(var, " ".join(val)) for var, val in flags.items()])

return config_args
11 changes: 10 additions & 1 deletion var/spack/repos/builtin/packages/nco/package.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,16 @@ class Nco(AutotoolsPackage):

def configure_args(self):
spec = self.spec
return ["--{0}-doc".format("enable" if "+doc" in spec else "disable")]

config_args = ["--{0}-doc".format("enable" if "+doc" in spec else "disable")]

@AlexanderRichert-NOAA AlexanderRichert-NOAA Feb 14, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor nitpick: I think this could be "self.enable_or_disable". Feel free to disregard.


# Older versions of the Intel compilers (definitely 18) can't compile
# nco with full optimization (-O2), internal compiler error.
if spec.satisfies("%intel@18"):
config_args.append("CFLAGS=-O1")
config_args.append("CXXFLAGS=-O1")

return config_args

def setup_build_environment(self, env):
spec = self.spec
Expand Down
11 changes: 9 additions & 2 deletions var/spack/repos/builtin/packages/netcdf-c/package.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class NetcdfC(AutotoolsPackage):
maintainers = ["skosukhin", "WardF"]

version("main", branch="main")
version("4.9.1", sha256="4ee8d5f6b50a1eb4ad4c10f24531e36261fd1882410fb08435eb2ddfd49a0908")
version("4.9.0", sha256="9f4cb864f3ab54adb75409984c6202323d2fc66c003e5308f3cdf224ed41c0a6")
version("4.8.1", sha256="bc018cc30d5da402622bf76462480664c6668b55eb16ba205a0dfb8647161dd0")
version("4.8.0", sha256="aff58f02b1c3e91dc68f989746f652fe51ff39e6270764e484920cb8db5ad092")
Expand Down Expand Up @@ -80,7 +81,11 @@ class NetcdfC(AutotoolsPackage):
variant("zstd", default=True, description="Enable ZStandard compression", when="@4.9.0:")
variant("optimize", default=True, description="Enable -O2 for a more optimized lib")
variant("nczarr", default=True, description="Enable zarr storage support", when="@4.8.0:")
variant("byterange", default=False, description="Allow byte-range I/O")
Comment thread
AlexanderRichert-NOAA marked this conversation as resolved.
# New byte-range I/O option doesn't compile with 4.9.1, will be fixed in 4.9.2
# https://github.com/Unidata/netcdf-c/issues/2614
variant("byterange", default=False, description="Allow byte-range I/O", when="@4.9.1")
variant("byterange", default=True, description="Allow byte-range I/O", when="@4.9.2:")

@AlexanderRichert-NOAA AlexanderRichert-NOAA Feb 14, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As long as we're at it, how about adding variant("byterange", default=False, description="Allow byte-range I/O", when="@4.7.0:4.9.0")? In any case I would lean toward setting it default=False because it's off by default in the build system itself (both configure and cmake).

@climbfuji climbfuji Feb 14, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I agree. I didn't know that it was available in 4.7.0 already. I'll make the change as follows (not that it goes up to 4.9.1):

    variant("byterange", default=False, description="Allow byte-range I/O", when="@4.7.0:4.9.1")
    variant("byterange", default=True, description="Allow byte-range I/O", when="@4.9.2:")

@climbfuji climbfuji Feb 14, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Very annoying, but for some reason the above suggestion makes +byterange the default, even for 4.9.0. Looks like you convinced me, for now I'll just use

variant("byterange", default=False, description="Allow byte-range I/O", when="@4.7.0:")

and we deal with enabling it for newer versions later.

# Variant fismahigh is required for FISMA high compliance on operational supercomputers
variant(
"fismahigh",
default=False,
Expand Down Expand Up @@ -152,7 +157,6 @@ class NetcdfC(AutotoolsPackage):
conflicts("+byterange", when="+fismahigh")
conflicts("+nczarr", when="+fismahigh")


filter_compiler_wrappers("nc-config", relative_root="bin")

@property
Expand Down Expand Up @@ -245,6 +249,9 @@ def configure_args(self):
else:
config_args.append("--disable-pnetcdf")

if "~byterange" in self.spec:
config_args.append("--disable-byterange")
Comment thread
climbfuji marked this conversation as resolved.
Outdated

if "+mpi" in self.spec or "+parallel-netcdf" in self.spec:
config_args.append("CC=%s" % self.spec["mpi"].mpicc)

Expand Down