Installer: rewire spliced specs via RewireTask by nhanford · Pull Request #39136 · spack/spack

nhanford · 2023-07-28T19:30:33Z

This PR allows users to configure explicit splicing replacement of an abstract spec in the concretizer.

concretizer:
  splice:
    explicit:
    - target: mpi
      replacement: mpich/abcdef
      transitive: true

This config block would mean "for any spec that concretizes to use mpi, splice in mpich/abcdef in place of the mpi it would naturally concretize to use. See #20262, #26873, #27919, and #46382 for PRs enabling splicing in the Spec object. This PR will be the first place the splice method is used in a user-facing manner. See https://spack.readthedocs.io/en/latest/spack.html#spack.spec.Spec.splice for more information on splicing.

This will allow users to reuse generic public binaries while splicing in the performant local mpi implementation on their system.

In the config file, the target may be any abstract spec. The replacement must be a spec that includes an abstract hash /abcdef. The transitive key is optional, defaulting to true if left out.

Two important items to note:

When writing explicit splice config, the user is in charge of ensuring that the replacement specs they use are binary compatible with whatever targets they replace. In practice, this will likely require either specific knowledge of what packages will be installed by the user's workflow, or somewhat more specific abstract "target" specs for splicing, to ensure binary compatibility.
Explicit splices can cause the output of the concretizer not to satisfy the input. For example, using the config above and consider a package in a binary cache hdf5/xyzabc that depends on mvapich2. Then the command spack install hdf5/xyzabc will instead install the result of splicing mpich/abcdef into hdf5/xyzabc in place of whatever mvapich2 spec it previously depended on. When this occurs, a warning message is printed `Warning: explicit splice configuration has caused the the concretized spec {concrete_spec} not to satisfy the input spec {input_spec}".

Highlighted technical details of implementation:

This PR required modifying the installer to have two separate types of Tasks, RewireTask and BuildTask. Spliced specs are queued as RewireTask and standard specs are queued as BuildTask. Each spliced spec retains a pointer to its build_spec for provenance. If a RewireTask is dequeued and the associated build_spec is neither available in the install_tree nor from a binary cache, the RewireTask is requeued with a new dependency on a BuildTask for the build_spec, and BuildTasks are queued for the build spec and its dependencies.
Relocation is modified so that a spack binary can be simultaneously installed and rewired. This ensures that installing the build_spec is not necessary when splicing from a binary cache.
The splicing model is modified to more accurately represent build dependencies -- that is, spliced specs do not have build dependencies, as spliced specs are never built. Their build_specs retain the build dependencies, as they may be built as part of installing the spliced spec.
There were vestiges of the compiler bootstrapping logic that were not removed in Deprecate config:install_missing_compilers #46237 because I asked @alalazo to leave them in to avoid making the rebase for this PR harder than it needed to be. Those last remains are removed in this PR.

becker33 · 2023-08-31T23:51:34Z

In case this rebase goes bad, the last commit before I attempted to rebase was 4cde3f7e4cc25e09be3907fcb0a27f6c2a10cb5f.

becker33 · 2023-09-05T18:52:58Z

TODO list:

docs
increase test coverage depending on codecov output
ensure all tests pass

becker33 · 2023-09-05T23:42:45Z

@spackbot fix style

spackbot-app · 2023-09-05T23:42:48Z

Let me see if I can fix that for you!

spackbot-app · 2023-09-05T23:44:07Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  lib/spack/spack/binary_distribution.py
  lib/spack/spack/config.py
  lib/spack/spack/environment/environment.py
  lib/spack/spack/installer.py
  lib/spack/spack/rewiring.py
  lib/spack/spack/schema/concretizer.py
  lib/spack/spack/schema/merged.py
  lib/spack/spack/schema/splice.py
  lib/spack/spack/solver/asp.py
  lib/spack/spack/spec.py
  lib/spack/spack/test/bindist.py
  lib/spack/spack/test/buildtask.py
  lib/spack/spack/test/concretize.py
  lib/spack/spack/test/conftest.py
  lib/spack/spack/test/installer.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted lib/spack/spack/test/concretize.py
All done! ✨ 🍰 ✨
1 file reformatted, 14 files left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
Success: no issues found in 586 source files
  mypy checks were clean
==> spack style checks were clean

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

But it looks like I'm not able to push to your branch. 😭️ Did you check Allow edits from maintainers when you opened the PR?

greenc-FNAL · 2023-09-27T17:13:18Z

[Adding here based on 2023-09-27 telcon discussion]

We have been using reuse concretization: allow externals from remote when locally configured #35975 in our local fork for several months now. If the same behavior was achievable with the functionality of this PR that would be welcome. Namely, that packages in BuildCache built with externals be considered for use at least on systems with compatible externals.
We have been hoping for some time that one could have a situation where a set of packages built for a generic microarchitecture (say x86_64v3) could be usable on a system where one or more packages should be built locally for the native microarchitecture while reusing downstream generic packages from BuildCache. Hypothetical: a DAG with multiple packages using numpy—BuildCache has the x86_64v3 builds, but we wish to have a compatible native build of numpy while still getting all the dependents thereof from BuildCache.

haampie · 2023-10-30T15:47:30Z

Can there be a human readable description of what this PR is doing?

tldahlgren

Very preliminary/minor feedback.

.github/workflows/valid-style.yml

lib/spack/spack/binary_distribution.py

lib/spack/spack/schema/splice.py

lib/spack/spack/binary_distribution.py

Co-authored-by: Todd Gamblin <[email protected]>

addressed

alalazo · 2024-10-11T06:38:51Z

lib/spack/spack/test/concretize.py

+
+        spec = spack.spec.Spec("hdf5 ^zmpi").concretized()
+
+        assert spec.satisfies(f"^mpich/{mpich_spec.dag_hash()}")


I'll submit a one line fix for this, but I think mpich dag hash is not an invariant when transitive=False. This test just happen to pass because currently mpich in the mock repository has no dependency, so I guess both transitive and non-transitive are behaving the same.

alalazo · 2024-10-11T07:26:52Z

lib/spack/spack/test/cmd/env.py

+    # The one spec is mpileaks
+    for _, spec in e2.concretized_specs():
+        assert spec.spliced
+        assert spec["mpi"].satisfies(zmpi)


I think even this assertion is not an invariant. If the original zmpi had a build dependency (in the mock repo it doesn't), then the replacement zmpi wouldn't have it - and that changes the hash of the node.

This PR allows users to configure explicit splicing replacement of an abstract spec in the concretizer. concretizer: splice: explicit: - target: mpi replacement: mpich/abcdef transitive: true This config block would mean "for any spec that concretizes to use mpi, splice in mpich/abcdef in place of the mpi it would naturally concretize to use. See spack#20262, spack#26873, spack#27919, and spack#46382 for PRs enabling splicing in the Spec object. This PR will be the first place the splice method is used in a user-facing manner. See https://spack.readthedocs.io/en/latest/spack.html#spack.spec.Spec.splice for more information on splicing. This will allow users to reuse generic public binaries while splicing in the performant local mpi implementation on their system. In the config file, the target may be any abstract spec. The replacement must be a spec that includes an abstract hash `/abcdef`. The transitive key is optional, defaulting to true if left out. Two important items to note: 1. When writing explicit splice config, the user is in charge of ensuring that the replacement specs they use are binary compatible with whatever targets they replace. In practice, this will likely require either specific knowledge of what packages will be installed by the user's workflow, or somewhat more specific abstract "target" specs for splicing, to ensure binary compatibility. 2. Explicit splices can cause the output of the concretizer not to satisfy the input. For example, using the config above and consider a package in a binary cache `hdf5/xyzabc` that depends on mvapich2. Then the command `spack install hdf5/xyzabc` will instead install the result of splicing `mpich/abcdef` into `hdf5/xyzabc` in place of whatever mvapich2 spec it previously depended on. When this occurs, a warning message is printed `Warning: explicit splice configuration has caused the the concretized spec {concrete_spec} not to satisfy the input spec {input_spec}". Highlighted technical details of implementation: 1. This PR required modifying the installer to have two separate types of Tasks, `RewireTask` and `BuildTask`. Spliced specs are queued as `RewireTask` and standard specs are queued as `BuildTask`. Each spliced spec retains a pointer to its build_spec for provenance. If a RewireTask is dequeued and the associated `build_spec` is neither available in the install_tree nor from a binary cache, the RewireTask is requeued with a new dependency on a BuildTask for the build_spec, and BuildTasks are queued for the build spec and its dependencies. 2. Relocation is modified so that a spack binary can be simultaneously installed and rewired. This ensures that installing the build_spec is not necessary when splicing from a binary cache. 3. The splicing model is modified to more accurately represent build dependencies -- that is, spliced specs do not have build dependencies, as spliced specs are never built. Their build_specs retain the build dependencies, as they may be built as part of installing the spliced spec. 4. There were vestiges of the compiler bootstrapping logic that were not removed in spack#46237 because I asked alalazo to leave them in to avoid making the rebase for this PR harder than it needed to be. Those last remains are removed in this PR. Co-authored-by: Nathan Hanford <[email protected]> Co-authored-by: Gregory Becker <[email protected]> Co-authored-by: Tamara Dahlgren <[email protected]>

spackbot-app bot added binary-packages core PR affects Spack core functionality tests General test capability(ies) labels Jul 28, 2023

spackbot-app bot added the stand-alone-tests Stand-alone (or smoke) tests for installed packages label Aug 11, 2023

spackbot-app bot added the environments label Aug 22, 2023

becker33 force-pushed the features/spliced-install branch from 4cde3f7 to 5e37abc Compare August 31, 2023 23:51

spackbot-app bot added the workflow label Sep 1, 2023

spackbot-app bot added dependencies new-version commands shell-support labels Sep 6, 2023

becker33 force-pushed the features/spliced-install branch from 4f4608c to 800451a Compare September 9, 2023 01:54

becker33 marked this pull request as ready for review September 9, 2023 01:54

becker33 changed the title ~~[WIP] Modify installer to install spliced specs~~ Installer: rewire spliced specs via RewireTask Sep 11, 2023

tgamblin added this to the v0.21.0 milestone Oct 12, 2023

tgamblin requested a review from becker33 October 17, 2023 18:02

tgamblin assigned becker33 Oct 17, 2023

tgamblin modified the milestones: v0.21.0, v0.22.0 Nov 6, 2023

tgamblin modified the milestones: v0.22.0, v0.23.0 Apr 29, 2024

becker33 force-pushed the features/spliced-install branch from bd8bdb8 to 77da5ce Compare June 28, 2024 17:02

tldahlgren reviewed Jun 29, 2024

View reviewed changes

.github/workflows/valid-style.yml Show resolved Hide resolved

lib/spack/spack/binary_distribution.py Outdated Show resolved Hide resolved

lib/spack/spack/schema/splice.py Outdated Show resolved Hide resolved

becker33 added 14 commits October 10, 2024 09:46

tighten up wording for comments and docstrings

eea2487

installer: consistent use of enum.auto()

cf40b3d

installer: document contract for Task.execute

3a43003

typing issue

9b8f7ad

relocation: only look at dependents within the spec

cbbd10c

remove redundant name-indexed spec access

f4aa02e

wip: fix dependency on name matching

230b69f

fixup after rebase

cbf2ac0

installer: remove all compiler bootstrapping

a3de96f

update test/buildtask.py to match

8d95927

rewire/bindist: find analogs use set instead of list

46c813e

Spec.splice: ensure spliced specs have no build deps

5fc54eb

explicit splice: warn on splice that deviates from input spec

5bdb606

fixup after rebase

c6823aa

becker33 force-pushed the features/spliced-install branch from f8cd521 to c6823aa Compare October 10, 2024 17:27

becker33 removed their request for review October 10, 2024 18:09

documentation: docs for explicit splices

0f65868

spackbot-app bot added the documentation Improvements or additions to documentation label Oct 10, 2024

allow trivial splices of the root spec

fdb7419

tgamblin requested changes Oct 10, 2024

View reviewed changes

lib/spack/spack/binary_distribution.py Outdated Show resolved Hide resolved

lib/spack/spack/binary_distribution.py Show resolved Hide resolved

deps_to_relocate: improve docstring

9868bc4

Co-authored-by: Todd Gamblin <[email protected]>

becker33 added 2 commits October 10, 2024 13:22

add comment on relocation analogs

fe09937

Add missing docs file

8cfc856

tgamblin approved these changes Oct 10, 2024

View reviewed changes

becker33 merged commit af62a06 into spack:develop Oct 10, 2024

alalazo reviewed Oct 11, 2024

View reviewed changes

alalazo mentioned this pull request Nov 1, 2024

Remove ignored config:install_missing_compilers from unit tests #47357

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Installer: rewire spliced specs via RewireTask#39136

Installer: rewire spliced specs via RewireTask#39136
becker33 merged 48 commits intospack:developfrom
nhanford:features/spliced-install

nhanford commented Jul 28, 2023 •

edited by becker33

Loading

Uh oh!

becker33 commented Aug 31, 2023

Uh oh!

becker33 commented Sep 5, 2023 •

edited

Loading

Uh oh!

becker33 commented Sep 5, 2023

Uh oh!

spackbot-app bot commented Sep 5, 2023

Uh oh!

spackbot-app bot commented Sep 5, 2023

Uh oh!

greenc-FNAL commented Sep 27, 2023

Uh oh!

haampie commented Oct 30, 2023

Uh oh!

tldahlgren left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo Oct 11, 2024

Uh oh!

alalazo Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants


		spec = spack.spec.Spec("hdf5 ^zmpi").concretized()

		assert spec.satisfies(f"^mpich/{mpich_spec.dag_hash()}")

Conversation

nhanford commented Jul 28, 2023 • edited by becker33 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

becker33 commented Aug 31, 2023

Uh oh!

becker33 commented Sep 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

becker33 commented Sep 5, 2023

Uh oh!

spackbot-app bot commented Sep 5, 2023

Uh oh!

spackbot-app bot commented Sep 5, 2023

Uh oh!

greenc-FNAL commented Sep 27, 2023

Uh oh!

haampie commented Oct 30, 2023

Uh oh!

tldahlgren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

alalazo Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nhanford commented Jul 28, 2023 •

edited by becker33

Loading

becker33 commented Sep 5, 2023 •

edited

Loading