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

Another test failure with KlyachkoBundle_class.random_deformation #32773

Open
kliem opened this issue Oct 26, 2021 · 17 comments
Open

Another test failure with KlyachkoBundle_class.random_deformation #32773

kliem opened this issue Oct 26, 2021 · 17 comments

Comments

@kliem
Copy link
Contributor

kliem commented Oct 26, 2021

Part of #32544:

Bug or incorrect test?

sage -t --long --random-seed=23747002002644886606785003174022326205 src/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "src/sage/schemes/toric/sheaf/klyachko.py", line 951, in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
Failed example:
    Vtilde.cohomology(dim=True, weight=(0,))
Expected:
    (1, 0)
Got:
    (0, 0)

CC: @vbraun @orlitzky @collares @sheerluck

Component: algebraic geometry

Author: Markus Wageringel

Branch/Commit: u/gh-mwageringel/32773 @ 27c8891

Issue created by migration from https://trac.sagemath.org/ticket/32773

@kliem kliem added this to the sage-9.5 milestone Oct 26, 2021
@dcoudert
Copy link
Contributor

dcoudert commented Dec 4, 2021

comment:1

Another one:

sage -t --long --warn-long 289.1 --random-seed=213487260798313884417459570000434714591 src/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "src/sage/schemes/toric/sheaf/klyachko.py", line 29, in sage.schemes.toric.sheaf.klyachko
Failed example:
    V.cohomology(dim=True, weight=(0,0,0,0))    # long time
Expected:
    (0, 0, 3, 0, 0)
Got:
    (0, 0, 0, 0, 0)

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mwageringel
Copy link

comment:4

The source of the problem seems to be the implementation of FilteredVectorSpace_class.random_deformation (source). This is the relevant code:

        filtration = dict()
        for deg, filt in self._filt[1:]:
            generators = [v + epsilon * random_vector(R, self.rank())
                          for v in filt.echelonized_basis()]
            filtration[deg] = generators

Occasionally, this random deformation results in generators that are linearly dependent, so that the new filtered component is of lower dimension than the original. It seems to me that this is not intended, as this will only happen in rare cases. Should we add a check here to avoid this?

@mwageringel
Copy link

Branch: u/gh-mwageringel/32773

@mwageringel
Copy link

comment:5

Here is an attempt at a partial fix which I think respects the original intent, while preserving the subspace inclusions and dimensions of the filtered vector spaces.

This fixes the issue in the description, but the test from comment:1 still sometimes fails:

sage -t --long --warn-long 69.2 --random-seed=298280877953805287150486292027026647714 src/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "src/sage/schemes/toric/sheaf/klyachko.py", line 29, in sage.schemes.toric.sheaf.klyachko
Failed example:
    V.cohomology(dim=True, weight=(0,0,0,0))    # long time
Expected:
    (0, 0, 3, 0, 0)
Got:
    (0, 0, 5, 0, 0)

New commits:

27c889132773: preserve dimensions and subspace inclusions in deformations

@mwageringel
Copy link

Commit: 27c8891

@slel
Copy link
Member

slel commented Feb 7, 2022

Author: Markus Wageringel

@mwageringel
Copy link

comment:7

If the current fix seems acceptable, I would suggest to open a new ticket for the remaining failure. We can mark it as known bug here to make the tests pass again.

@orlitzky
Copy link
Contributor

comment:8

It looks like all of this machinery was added specifically for Klyachko bundles, so it really comes down to the intent. And I think the FilteredVectorSpace constructor guarantees the subspace inclusions? If so, it's only the dimensions of the components that can cause an issue; I don't see a problem with that a priori for general filtered vector spaces, but the docs for the Klyachko bundle method say,

Return a generic torus-equivariant deformation of the bundle... A new Klyachko bundle with randomly perturbed moduli. In particular, the same Chern classes.

and I have to admit that I don't know what very many of those words mean. If it's only the Klyachko class that needs the dimensions of the components to remain invariant, it might be better to change

while not filt.is_exhaustive():
    filt = self._filt.random_deformation(epsilon)

to preserve those dimensions as well.

@mwageringel
Copy link

comment:9

Yes, I think your assessments are correct. I am not completely sure about the original intent either.

@vbraun: Could you help us on this, please? You seem to be the original author. In particular, what is needed for the random deformations of Klyachko bundles to have those properties? And should random deformations of filtered vector spaces preserve the dimensions of their components?

@dimpase
Copy link
Member

dimpase commented Apr 4, 2022

comment:10

it seems to me that "random" here silently means "generic", as well.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@strogdon
Copy link

comment:13

This also happens with a different random-seed.

sage -t --long --warn-long 91.4 --random-seed=194967010328380092288236333293077030743 /usr/lib/python3.10/site-packages/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/schemes/toric/sheaf/klyachko.py", line 951, in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
Failed example:
    Vtilde.cohomology(dim=True, weight=(0,))
Expected:
    (1, 0)
Got:
    (0, 0)
**********************************************************************
1 item had failures:
   1 of   7 in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
    [151 tests, 1 failure, 7.14 s]
----------------------------------------------------------------------
sage -t --long --warn-long 91.4 --random-seed=19496701032838009228823633

@strogdon
Copy link

comment:14

Replying to @strogdon:

This also happens with a different random-seed.

sage -t --long --warn-long 91.4 --random-seed=194967010328380092288236333293077030743 /usr/lib/python3.10/site-packages/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "/usr/lib/python3.10/site-packages/sage/schemes/toric/sheaf/klyachko.py", line 951, in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
Failed example:
    Vtilde.cohomology(dim=True, weight=(0,))
Expected:
    (1, 0)
Got:
    (0, 0)
**********************************************************************
1 item had failures:
   1 of   7 in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
    [151 tests, 1 failure, 7.14 s]
----------------------------------------------------------------------
sage -t --long --warn-long 91.4 --random-seed=19496701032838009228823633

This was on Sage-on-Gentoo. With vanilla sage the failure is

sage -t --long --warn-long 91.4 --random-seed=194967010328380092288236333293077030743 src/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "src/sage/schemes/toric/sheaf/klyachko.py", line 951, in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
Failed example:
    Vtilde.cohomology(dim=True, weight=(0,))
Expected:
    (1, 0)
Got:
    (0, 0)
**********************************************************************
1 item had failures:
   1 of   7 in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
    [151 tests, 1 failure, 7.30 s]
----------------------------------------------------------------------
sage -t --long --warn-long 91.4 --random-seed=194967010328380092288236333293077030743 src/sage/schemes/toric/sheaf/klyachko.py  # 1 doctest failed

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@cheuberg
Copy link
Contributor

cheuberg commented Jan 2, 2023

comment:16

Another one (in plain 9.8.beta6)

sage -t --long --warn-long 48.2 --random-seed=20117131592758165284897735586849676785 src/sage/schemes/toric/sheaf/klyachko.py
**********************************************************************
File "src/sage/schemes/toric/sheaf/klyachko.py", line 951, in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
Failed example:
    Vtilde.cohomology(dim=True, weight=(0,))
Expected:
    (1, 0)
Got:
    (0, 0)
**********************************************************************
1 item had failures:
   1 of   7 in sage.schemes.toric.sheaf.klyachko.KlyachkoBundle_class.random_deformation
    [151 tests, 1 failure, 6.59 s]

@dcoudert
Copy link
Contributor

dcoudert commented Jan 2, 2023

comment:17

Do we have anyone able to review this ticket ? otherwise we can call for help on sage-devel.

@dimpase
Copy link
Member

dimpase commented Jan 5, 2023

comment:18

Volker is the main author of this - he's already in cc, though.

@dimpase
Copy link
Member

dimpase commented Jan 5, 2023

comment:19

There was already attempt to fix this on #29956
Adding the author of the latter to CC.

@tornaria
Copy link
Contributor

tornaria commented Feb 9, 2023

While this is resolved, could we flag this test as # random, please?

tornaria added a commit to tornaria/sage that referenced this issue Feb 10, 2023
tornaria added a commit to tornaria/sage that referenced this issue Feb 10, 2023
dimpase pushed a commit to tornaria/sage that referenced this issue Feb 14, 2023
dimpase added a commit that referenced this issue Feb 14, 2023
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description

These two test failures occasionally waste a whole CI run. Here we just
flag them as random so it can be quickly merged, while #32773 is
resolved. Since the issue is already known and reported, there is no
point to keep running these tests.

Note that the issue in question has a needs_review patch but according
to the comments it only solves one of the two tests.

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [X] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [X] I have linked an issue or discussion.


<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants