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

Fix performance issue in hilbn and friends #9

Closed
wants to merge 1 commit into from

Conversation

8d1h
Copy link

@8d1h 8d1h commented Aug 3, 2021

Hi! I was playing with the Hodge diamonds of K3^[n] and I noticed there is a huge performance issue. It appears that power series does not allow access of individual coefficients, and series.coefficients() creates a large dictionary which contains a lot of redundant info. So it's a lot faster to convert a power series to a polynomial then access its coefficients.

The same issue happens in nestedhilbn and complete_intersection too.

Also I fixed the variable names in cyclic_cover :)

* also fix bug in cyclic_cover
@pbelmans
Copy link
Owner

pbelmans commented Aug 3, 2021

Thanks! I knew it wasn't very efficient, but it served it purpose for things of the size that I was interested in so far :).

I've been running a documentation-adding branch offline, that I need to finish up asap. I'll include your changes there, unless an upstream merge happens to still be possible somehow.

@8d1h
Copy link
Author

8d1h commented Aug 3, 2021

Sure!

pbelmans added a commit that referenced this pull request Aug 4, 2021
@pbelmans
Copy link
Owner

pbelmans commented Aug 4, 2021

I have now finally finished writing documentation and unit tests, and after that I could copy-paste your changes and quickly verify that all unit tests still pass :). Hurray for unit tests! But it sure was a tedious task...

The automated testing went from 18s or so to 5s.

For those who want a sneak peek of the documentation, check out https://pbelmans.ncag.info/hodge-diamond-cutter/

@pbelmans pbelmans closed this Aug 4, 2021
@8d1h
Copy link
Author

8d1h commented Aug 5, 2021

The automated testing went from 18s or so to 5s.

Yes already hilbn(K3, 5) was very slow and I think your tests computed it twice... Now we can easily compute K3^[10] :)

BTW, although I doubt anyone would really want to compute this, but for abelian(g) with large g it is a lot faster using curve(1)^g instead.

sage: time a = abelian(100)
CPU times: user 15.2 s, sys: 0 ns, total: 15.2 s
Wall time: 15.2 s
sage: time b = curve(1)^100
CPU times: user 329 ms, sys: 0 ns, total: 329 ms
Wall time: 329 ms
sage: a == b
True

For those who want a sneak peek of the documentation, check out https://pbelmans.ncag.info/hodge-diamond-cutter/

This is really nice! :)

@pbelmans
Copy link
Owner

pbelmans commented Aug 5, 2021

Excellent point about the computation for abelian varieties! This is now the computation method. Thanks!

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