Skip to content

Commit

Permalink
Collapse redundant dependency clauses enumerating available versions (#…
Browse files Browse the repository at this point in the history
…6160)

In #5046, we show the tautological
proof:

```
  ╰─▶ Because colabfold[alphafold]==1.5.5 depends on jax>=0.4.20 and only the following versions of jax are available:
          jax<=0.4.20
          jax==0.4.21
          jax==0.4.22
          jax==0.4.23
          jax==0.4.24
          jax==0.4.25
          jax==0.4.26
          jax==0.4.27
          jax==0.4.28
          jax==0.4.29
          jax==0.4.30
          jax==0.4.31
      we can conclude that colabfold[alphafold]==1.5.5 depends on jax>=0.4.20.
      And because jax>=0.4.20 depends on numpy>=1.26.0, we can conclude that colabfold[alphafold]==1.5.5 depends on numpy>=1.26.0.
      (1)
```

This is a part of the error tree because the statement
`colabfold[alphafold]==1.5.5 depends on jax>=0.4.20` is actually a
simplification of `colabfold[alphafold]==1.5.5 depends on
jax>=0.4.20,<0.5.0` and the no versions clause is a proof of that
simplification.

Without simplification, the clause looks like:

```
  ╰─▶ Because colabfold[alphafold]==1.5.5 depends on jax>=0.4.20,<0.5.0 and only the following versions of jax are available:
          jax<=0.4.20
          jax==0.4.21
          jax==0.4.22
          jax==0.4.23
          jax==0.4.24
          jax==0.4.25
          jax==0.4.26
          jax==0.4.27
          jax==0.4.28
          jax==0.4.29
          jax==0.4.30
          jax==0.4.31
      we can conclude that colabfold[alphafold]==1.5.5 depends on one of:
          jax==0.4.20
          jax==0.4.21
          jax==0.4.22
          jax==0.4.23
          jax==0.4.24
          jax==0.4.25
          jax==0.4.26
          jax==0.4.27
          jax==0.4.28
          jax==0.4.29
          jax==0.4.30
          jax==0.4.31
      And because jax>=0.4.20 depends on numpy>=1.26.0, we can conclude that colabfold[alphafold]==1.5.5 depends on numpy>=1.26.0.
```

I don't think we have a great way to avoid performing the simplification
of the range conditionally and it makes the error simpler to just jump
straight to `colabfold[alphafold]==1.5.5 depends on jax>=0.4.20`.

The derivation for this clause looks like:

```
          jax==0.4.20 | ==0.4.21 | ==0.4.22 | ==0.4.23 | ==0.4.24 | ==0.4.25 | ==0.4.26 | ==0.4.27 | ==0.4.28 | ==0.4.29 | ==0.4.30 | ==0.4.31 depends on numpy>=1.26.0
            no versions of jax>0.4.20, <0.4.21 | >0.4.21, <0.4.22 | >0.4.22, <0.4.23 | >0.4.23, <0.4.24 | >0.4.24, <0.4.25 | >0.4.25, <0.4.26 | >0.4.26, <0.4.27 | >0.4.27, <0.4.28 | >0.4.28, <0.4.29 | >0.4.29, <0.4.30 | >0.4.30, <0.4.31 | >0.4.31, <0.5.0
            colabfold[alphafold]==1.5.5 depends on jax>=0.4.20, <0.5.0
```

So it looks like we can take trees of this form and drop the "no
versions" clause _if_ the ranges are compatible[*]. See [this
comment](#6160 (comment))
for a simpler explanation.

With this pull request, the clause simplifies to

```
╰─▶ Because colabfold[alphafold]==1.5.5 depends on jax>=0.4.20 and jax>=0.4.20 depends on numpy>=1.26.0,
     we can conclude that colabfold[alphafold]==1.5.5 depends on numpy>=1.26.0. (1)
```

Unfortunately, this doesn't change any snapshots in our test suite so
I'm uncertain if the strategy generalizes. In some incorrect iterations
of this logic, the snapshots did reveal my mistakes.

[*] "if the ranges are compatible" includes a bit of hand-waving. I'm
not 100% sure if I've chosen the correct range heuristic here.
  • Loading branch information
zanieb authored Aug 19, 2024
1 parent df2ebf7 commit f6f2c5b
Showing 1 changed file with 102 additions and 0 deletions.
102 changes: 102 additions & 0 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ impl std::fmt::Display for NoSolutionError {
}

collapse_unavailable_versions(&mut tree);
collapse_redundant_depends_on_no_versions(&mut tree);

if should_display_tree {
display_tree(&tree, "Resolver derivation tree after reduction");
Expand Down Expand Up @@ -364,6 +365,107 @@ fn collapse_no_versions_of_workspace_members(
}
}

/// Given a [`DerivationTree`], collapse `NoVersions` incompatibilities that are redundant children
/// of a dependency. For example, if we have a tree like:
///
/// A>=1,<2 depends on B
/// A has no versions >1,<2
/// C depends on A>=1,<2
///
/// We can simplify this to `C depends A>=1 and A>=1 depends on B so C depends on B` without
/// explaining that there are no other versions of A. This is dependent on range of A in "A depends
/// on" being a subset of range of A in "depends on A". For example, in a tree like:
///
/// A>=1,<3 depends on B
/// A has no versions >2,<3
/// C depends on A>=2,<3
///
/// We cannot say `C depends on A>=2 and A>=1 depends on B so C depends on B` because there is a
/// hole in the range — `A>=1,<3` is not a subset of `A>=2,<3`.
fn collapse_redundant_depends_on_no_versions(
tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
) {
match tree {
DerivationTree::External(_) => {}
DerivationTree::Derived(derived) => {
// If one node is a dependency incompatibility...
match (
Arc::make_mut(&mut derived.cause1),
Arc::make_mut(&mut derived.cause2),
) {
(
DerivationTree::External(External::FromDependencyOf(package, versions, _, _)),
ref mut other,
)
| (
ref mut other,
DerivationTree::External(External::FromDependencyOf(package, versions, _, _)),
) => {
// Check if the other node is the relevant form of subtree...
collapse_redundant_depends_on_no_versions_inner(other, package, versions);
}
// If not, just recurse
_ => {
collapse_redundant_depends_on_no_versions(Arc::make_mut(&mut derived.cause1));
collapse_redundant_depends_on_no_versions(Arc::make_mut(&mut derived.cause2));
}
}
}
}
}

/// Helper for [`collapse_redundant_depends_on_no_versions`].
fn collapse_redundant_depends_on_no_versions_inner(
tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
package: &PubGrubPackage,
versions: &Range<Version>,
) {
match tree {
DerivationTree::External(_) => {}
DerivationTree::Derived(derived) => {
// If we're a subtree with dependency and no versions incompatibilities...
match (&*derived.cause1, &*derived.cause2) {
(
DerivationTree::External(External::NoVersions(no_versions_package, _)),
dependency_clause @ DerivationTree::External(External::FromDependencyOf(
_,
_,
dependency_package,
dependency_versions,
)),
)
| (
dependency_clause @ DerivationTree::External(External::FromDependencyOf(
_,
_,
dependency_package,
dependency_versions,
)),
DerivationTree::External(External::NoVersions(no_versions_package, _)),
)
// And these incompatibilities (and the parent incompatibility) all are referring to
// the same package...
if no_versions_package == dependency_package
&& package == no_versions_package
// And parent dependency versions are a subset of the versions in this tree...
&& versions.subset_of(dependency_versions) =>
{
// Enumerating the available versions will be redundant and we can drop the no
// versions clause entirely in favor of the dependency clause.
*tree = dependency_clause.clone();

// Note we are at a leaf of the tree so there's no further recursion to do
}
// If not, just recurse
_ => {
collapse_redundant_depends_on_no_versions(Arc::make_mut(&mut derived.cause1));
collapse_redundant_depends_on_no_versions(Arc::make_mut(&mut derived.cause2));
}
}
}
}
}

/// Given a [`DerivationTree`], collapse incompatibilities for versions of a package that are
/// unavailable for the same reason to avoid repeating the same message for every unavailable
/// version.
Expand Down

0 comments on commit f6f2c5b

Please sign in to comment.