Weil divisors #1734
Weil divisors #1734
Conversation
|
Something goes wrong with CC: @HereAround |
|
What about #1585? |
|
Someone is using |
Benjamin pointed out that |
|
Hm, I don't think that adjoint method should exist, see Nemocas/AbstractAlgebra.jl#838 |
|
Ok, so the offending line is in Before I used |
|
@lkastner Thanks for the explanation and no worries. I will adjust it in a separate PR. |
|
On Wed, Nov 16, 2022 at 02:37:12PM -0800, Lars Kastner wrote:
Ok, so the offending line is
```
tr = adjoint(lattice_gens)
```
in `src/ToricVarities/NormalToricVarieties/constructors.jl`. The
reason I was using this is that `lattice_gens` is a `fmpz_mat` and
hence cannot be inverted. So I thought that `inv=adj/det`, but I just
looked at wikipedia and apparently this is an outdated understanding
of adjoint.
Why? if m has det==1, inv will work
otherwise, try pseudo_inv which will return a matrix and a scalar
(denominator)
…
Before I used `solve` multiple time which I found inelegant. The adjoint or rather adjunct works because I only need ray generators, a scalar factor does not matter. This is just to explain what this was supposed to accomplish. Let me know how you want to proceed / whether I need to do something.
--
Reply to this email directly or view it on GitHub:
#1734 (comment)
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
|
Yes, I was already worried about this. Indeed, some time ago, I wanted to have a toy-implementation of inversion of matrices over rings and I implemented the |
It's a remake of that PR. Issues you pointed out there should eventually be addressed again. I'll look into that. |
Exactly, in my case |
|
Ok, I switched to |
src/Rings/mpolyquo-localizations.jl
Outdated
| @Markdown.doc """ | ||
| is_integral_domain(R::Ring) | ||
|
|
||
| Return whether or not `R` is an integral domain. | ||
| """ | ||
| function is_integral_domain(R::Ring) | ||
| is_domain_type(typeof(R)) && return true | ||
| error("method not implemented for rings of type $(typeof(R))") | ||
| end |
There was a problem hiding this comment.
@thofma @fieker @fingolfin : This would be my implementation of the is_integral_domain thing. The other (implemented) methods are scattered throughout the files in this commit.
Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>
…l into weil_divisors_new
| I_powers = [(1,I)] | ||
|
|
||
| while !P(last(I_powers)[2]) | ||
| push!(I_powers, (last(I_powers)[1]*2, last(I_powers)[2]^2)) |
There was a problem hiding this comment.
Doubling the power with each iteration could be dangerous if
I^16 is just too difficult but I^9 does the trick.
There was a problem hiding this comment.
Usually, I would expect small orders of vanishing and hence I, I^2, I^3,... should suffice?
There was a problem hiding this comment.
This was following a hint by @fieker from a long time ago in another context. I see your point, but would vote for first keeping an eye on this in practice. We do not need to compute a Groebner basis of the power, but 'only' a saturation. I'm not sure whether that makes things easier, but it could be possible.
If it turns out that this is where stuff gets stuck, we can switch. But in general, this approach saves us a lot of time since it reduces n computations to 2*log(n) computations. Maybe, we have to introduce another heuristic at some point, depending on the complexity of the input, to go incrementally from some power onwards. Let's see.
Co-authored-by: simonbrandhorst <51749255+simonbrandhorst@users.noreply.github.com>
Co-authored-by: simonbrandhorst <51749255+simonbrandhorst@users.noreply.github.com>
Co-authored-by: simonbrandhorst <51749255+simonbrandhorst@users.noreply.github.com>
…l into weil_divisors_new
| A = zero(MatrixSpace(L, ngens(Jsat), ngens(I))) | ||
| for i in 1:length(cache) | ||
| (g, a, dttk) = cache[i] | ||
| A[i, :] = L(one(dttk), dttk, check=false)*change_base_ring(L, a)*pre_saturation_data(I) | ||
| end | ||
| I.pre_saturated_ideal = Jsat | ||
| I.pre_saturation_data = A |
There was a problem hiding this comment.
This needs a test.
There was a problem hiding this comment.
Then I would merge it :-).
There was a problem hiding this comment.
Then I would merge it
But with a squash, please. It's become too messy.
This is a new draft PR for Weil divisors and their functionality for discussion and further development.
The internals have changed in the sense that we now build on the new
IdealSheafderived from the presheaf container.CC: @simonbrandhorst