Skip to content

solver: simplify encoding of versions#51591

Merged
alalazo merged 5 commits intospack:developfrom
alalazo:solver/simplify-version-weights
Nov 17, 2025
Merged

solver: simplify encoding of versions#51591
alalazo merged 5 commits intospack:developfrom
alalazo:solver/simplify-version-weights

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Nov 17, 2025

Due to historical reasons, in develop we are encoding version data as a triplet of:

  1. The version itself
  2. A possible index for that version (used to sort it later)
  3. The origin of the version

In time, we added more version origins that do not need an index. Also, the EXTERNAL origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:

  • Each version has a single version weight, regardless of its origin
  • Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

Encoding before

pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).

Encoding after

pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).

Before this commit, the same version for a package was penalized
differently depending on its origin.

Now each version has a single weight, and can be associated with
multiple origins.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Massimiliano Culpo <[email protected]>
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 17, 2025

Benchmarked against

radiuss.develop.csv
radiuss.pr.csv

comparison

@alalazo alalazo marked this pull request as ready for review November 17, 2025 13:42
Copy link
Copy Markdown
Member

@haampie haampie left a comment

Choose a reason for hiding this comment

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

lgtm

@alalazo alalazo merged commit 7a06883 into spack:develop Nov 17, 2025
31 of 32 checks passed
@alalazo alalazo deleted the solver/simplify-version-weights branch November 17, 2025 15:18
psakievich pushed a commit to psakievich/spack that referenced this pull request Dec 1, 2025
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins 

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
harshula pushed a commit to ACCESS-NRI/spack that referenced this pull request Dec 9, 2025
* Cherry-picked upstream commit 7a06883

Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
haampie added a commit that referenced this pull request Dec 15, 2025
haampie added a commit that referenced this pull request Dec 15, 2025
This reverts commit 7a06883.

Signed-off-by: Harmen Stoppels <[email protected]>
haampie added a commit that referenced this pull request Dec 15, 2025
This reverts commit 7a06883.

Signed-off-by: Harmen Stoppels <[email protected]>
haampie pushed a commit that referenced this pull request Dec 16, 2025
#51591 started applying deprecation penalties also to external specs. Here we restore  the previous semantics and exclude externals from taking a deprecation penalty.

This is done applying a separate:

```prolog
deprecation_penalty(PackageNode, Penalty).
```

in the ASP problem, according to certain rules. This makes it easier to debug whether a penalty for deprecation was taken, and to add or modify behavior around deprecation  in following PRs.


Signed-off-by: Massimiliano Culpo <[email protected]>
@becker33 becker33 mentioned this pull request Jan 11, 2026
2 tasks
becker33 pushed a commit that referenced this pull request Jan 11, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins 

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 11, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 12, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 15, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 16, 2026
#51591 started applying deprecation penalties also to external specs. Here we restore  the previous semantics and exclude externals from taking a deprecation penalty.

This is done applying a separate:

```prolog
deprecation_penalty(PackageNode, Penalty).
```

in the ASP problem, according to certain rules. This makes it easier to debug whether a penalty for deprecation was taken, and to add or modify behavior around deprecation  in following PRs.


Signed-off-by: Massimiliano Culpo <[email protected]>
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
spack#51591 started applying deprecation penalties also to external specs. Here we restore  the previous semantics and exclude externals from taking a deprecation penalty.

This is done applying a separate:

```prolog
deprecation_penalty(PackageNode, Penalty).
```

in the ASP problem, according to certain rules. This makes it easier to debug whether a penalty for deprecation was taken, and to add or modify behavior around deprecation  in following PRs.


Signed-off-by: Massimiliano Culpo <[email protected]>
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
This reverts commit 729c685.

Signed-off-by: Harmen Stoppels <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 22, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 22, 2026
#51591 started applying deprecation penalties also to external specs. Here we restore  the previous semantics and exclude externals from taking a deprecation penalty.

This is done applying a separate:

```prolog
deprecation_penalty(PackageNode, Penalty).
```

in the ASP problem, according to certain rules. This makes it easier to debug whether a penalty for deprecation was taken, and to add or modify behavior around deprecation  in following PRs.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 26, 2026
Due to historical reasons, in `develop` we are encoding version data as a triplet of:

1. The version itself
2. A possible index for that version (used to sort it later)
3. The origin of the version

In time, we added more version origins that _do not need_ an index. Also, the `EXTERNAL` origin is not needed anymore, since externals are now treated like any other installed spec.

This PR, therefore, simplifies the encoding of versions in the solver so that:
- Each version has a single version weight, regardless of its origin
- Each version may be associated with multiple origins

It also removes unneeded data structures in the code.

**Encoding before**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0,"package_py")).
pkg_fact("mpileaks",version_declared("2.2",1,"package_py")).
pkg_fact("mpileaks",version_declared("2.1",2,"package_py")).
pkg_fact("mpileaks",version_declared("1.0",3,"package_py")).
pkg_fact("mpileaks",version_declared("2.3",4,"installed")).
```

**Encoding after**
```
pkg_fact("mpileaks",namespace("builtin_mock")).
pkg_fact("mpileaks",version_declared("2.3",0)).
pkg_fact("mpileaks",version_origin("2.3","installed")).
pkg_fact("mpileaks",version_origin("2.3","package_py")).
pkg_fact("mpileaks",version_declared("2.2",1)).
pkg_fact("mpileaks",version_origin("2.2","package_py")).
pkg_fact("mpileaks",version_declared("2.1",2)).
pkg_fact("mpileaks",version_origin("2.1","package_py")).
pkg_fact("mpileaks",version_declared("1.0",3)).
pkg_fact("mpileaks",version_origin("1.0","package_py")).
```

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
becker33 pushed a commit that referenced this pull request Jan 26, 2026
#51591 started applying deprecation penalties also to external specs. Here we restore  the previous semantics and exclude externals from taking a deprecation penalty.

This is done applying a separate:

```prolog
deprecation_penalty(PackageNode, Penalty).
```

in the ASP problem, according to certain rules. This makes it easier to debug whether a penalty for deprecation was taken, and to add or modify behavior around deprecation  in following PRs.

Signed-off-by: Massimiliano Culpo <[email protected]>
Signed-off-by: Gregory Becker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants