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

Sheaf cohomology tables and potential rework of Betti tables #2392

Merged
merged 18 commits into from
Sep 15, 2023

Conversation

RafaelDavidMohr
Copy link
Contributor

This PR will implement the Oscar interface to the Singular function sheafCohBGG:

https://www.singular.uni-kl.de/Manual/4-3-1/sing_1827.htm#SEC1908

Me and @ederc found that the easiest way to get the printing of the sheaf cohomology tables done as in Singular would be to use PrettyTables.jl. This package is already present as a test dependency for Oscar. If we print these tables using this package then it would have to be added as a "proper" dependency.

If we decide to do use PrettyTables.jl then we could also use it to print Betti tables which would simplify the Base.show implementation for Betti Tables (see the Base.show implementation on this branch for a "preview" of what this could look like). Without pulling in extra dependencies, PrettyTables.jl requires the data to be present in a matrix format in order to print it. As a consequence, if @jankoboehm agrees to using PrettyTables.jl for Betti tables, then I would also propose reworking the Betti table data structure to store it directly as a matrix and to also potentially implement a Base.getindex method for Betti tables so that the user may easily query the tables.

Comment on lines 978 to 979
grade_group = parent(first(keys(bt_table))[2])
if !(isone(ngens(grade_group)) && isone(nrels(grade_group)))
Copy link
Contributor

Choose a reason for hiding this comment

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

is_z_graded

@thofma
Copy link
Collaborator

thofma commented May 16, 2023

I would suggest that we do not add a dependency on PrettyTables.jl.

@ederc
Copy link
Member

ederc commented May 16, 2023

I would suggest that we do not add a dependency on PrettyTables.jl.

Why exactly? Doing all these tables by hand, as it is implemented right now, is not a good way to do. What would be your suggestion?

@thofma
Copy link
Collaborator

thofma commented May 16, 2023

External dependencies which we do not control are bad. There was recently another fiasco, where a maintainer of heavily used packages went missing (https://github.com/JuliaFolds).

@jankoboehm
Copy link
Contributor

jankoboehm commented May 16, 2023

The data structure of the Betti table as a Dict has an important mathematical meaning in the form it is, so it should please not be changed.

I do not mind a different show function, as long as it prints as currently, which is exactly what we want.

That show function would first have to generate internally a matrix and then print it. I would however also suggest that we are very cautious to add a dependency, since we can pretty easily produce our own generic code to handle the pretty printing.

In case we would want to go into this deeper, there is the very nice conpept of a Net (which is modular and can be used for any sort of printing for tables, matrices, polynomials, maps, rings ...) in M2 and Singular.

@RafaelDavidMohr
Copy link
Contributor Author

The data structure of the Betti table as a Dict has an important mathematical meaning in the form it is, so it should please not be changed.

I do not mind a different show function, as long as it prints as currently, which is exactly what we want.

That show function would first have to generate internally a matrix and then print it. I would however also suggest that we are very cautious to add a dependency, since we can pretty easily produce our own generic code to handle the pretty printing.

In case we would want to go into this deeper, there is the very nice conpept of a Net (which is modular and can be used for any sort of printing for tables, matrices, polynomials, maps, rings ...) in M2 and Singular.

Ok, thanks for weighing in. In light of this and the above discussion I will leave the Betti table completely as is and simply also use its Base.show implementation for the sheaf cohomology tables so as to not add the PrettyTables dependency.

Just FYI, they are currently on hold due to an issue on the Singular side.

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #2392 (ea6dce8) into master (79874fd) will decrease coverage by 0.06%.
Report is 9 commits behind head on master.
The diff coverage is 64.28%.

@@            Coverage Diff             @@
##           master    #2392      +/-   ##
==========================================
- Coverage   73.63%   73.57%   -0.06%     
==========================================
  Files         455      455              
  Lines       64411    64517     +106     
==========================================
+ Hits        47427    47467      +40     
- Misses      16984    17050      +66     
Files Changed Coverage
experimental/JuLie/src/schur_polynomials.jl ø
...ntal/QuadFormAndIsom/src/lattices_with_isometry.jl ø
experimental/Schemes/CoveredProjectiveSchemes.jl ø
src/AlgebraicGeometry/Schemes/Glueing/Methods.jl ø
...eometry/Schemes/ProjectiveSchemes/Objects/Types.jl ø
src/Modules/FreeModules-graded.jl 0.00%
src/Rings/mpoly-affine-algebras.jl 0.00%
src/Rings/mpoly-graded.jl 0.00%
src/Rings/mpolyquo-localizations.jl ø
src/TropicalGeometry/curve.jl 0.00%
... and 12 more

@fingolfin
Copy link
Member

What's the status of this? Is the Singular issue resolved? If not, please add a link to the relevant issue at the Singular.jl and or Singular repository, so we can track its status

@wdecker
Copy link
Collaborator

wdecker commented Aug 22, 2023 via email

@RafaelDavidMohr
Copy link
Contributor Author

I added a first Wrapper for the sheafCohBGGregul_w function. Let me know if this is going in the right direction. Also, there still seem to be some issues on the Singular side, see Singular/Singular#1184

@RafaelDavidMohr RafaelDavidMohr marked this pull request as ready for review September 12, 2023 08:38
@wdecker
Copy link
Collaborator

wdecker commented Sep 13, 2023

I have several problems here. In particular, the results seem to be incorrect.

@RafaelDavidMohr, @ederc: Can we discuss this in gathertown some time soon?

For example, Tomorrow, 14.10., 11:00?

@wdecker wdecker merged commit f6253d7 into oscar-system:master Sep 15, 2023
13 of 15 checks passed
fieker pushed a commit that referenced this pull request Sep 29, 2023
* first sketch for print code

* fixes header alignment

* finishes printing code

* first version of sheaf coh bgg

* make it work for free modules

* add indexing method for sheaf coh table

* export

* add some initial docu

* fix extraction of cokernel repr

* get weights correctly

* cast weight to int

* change name, take out regularity

* new example in docu

* add free module docu example

* add some sheaf coh tests

* remove error message (already covered by type)

* add error message back in, adjust test instead

* fix tests

---------

Co-authored-by: Rafael Mohr <[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.

7 participants