Update I.dim when dimension computed

[rprebet]

No description provided.

[rprebet]

The tests failure does not come from my commits. They occur even when running tests on the current version of the repo on my computer.

I suspect that the problem comes from the change of version of msolve and compatibility problem(s).

[rprebet]

I have updated the tests according to the output of the current binary of msolve 0.7.501.
It seems that there have been enough changes to the isolation that the output has changed significantly.

This also comes with an observation: in this test, with the previous version, some coordinates were 0 because the isolation intervals were of the form [-1/2^n, 1/2^n]. This is no longer the case with the new version.
Unless they are non-zero (which would be surprising), is this behaviour desirable? If not, I can add it as an issue to the msolve repo.

[mohabsafey]

Many thanks. I will have a closer look asap.
Indeed, I changed recently the output of the real root extraction. For some examples, msolve was indeed returning intervals [-1.2^k,1/2^k] while, internally, it was clear that the true value is 0. In this cases, it should return [0,0] which is desirable.
A fact which is not so satisfactory is that sometimes, to detect that some coordinate is actually zero, one needs some extra calculations in msolve which are not implemented yet. This is on my (long) todo list.
More news on this PR later

[rprebet]

Thank you for your fast reply! This is clearer now.

However, do you think it would be possible to accept this PR in the meantime?
Indeed, the current version of the repo does not pass the tests and in particular prevents from doing any other PR.

[rprebet]

Commit [26035e0] is questionable, as one might want to store the information that an ideal has a positive dimension.
However, with the current code, it would say that any positive dimensional Ideal has dimension 1.
Another solution would be to have another field in Ideals type.

[ederc]

I'm fine with this PR, let's see what @mohabsafey further wants to say.
Nevertheless this is a breaking change and we need a new major release after this.

[rprebet]

I have a few other (much more substantial) pending PRs to submit.
In particular, the next one concerns Hilbert series computations perhaps you'd like to include it in the next major release?
I can submit it now if you want.

[mohabsafey]

Initially, the dimension field was only there to indicate if the ideal has dimenion <= 0.
Then, shouldn't we replace dim with .iszerodim or .isfinite which would be set to True if the ideal has dimension <=0 and False otherwise and then bring back .dim once we have Hilbert Series in AlgebraicSolving ?
Otherwise, I am fine with merging this PR and make these deeper changes later.

[rprebet]

I think .dim could also have been used to indicate that the ideal is not proper if it is -1. However, there would be an ambiguity (to fix) since the default value of .dim is -1. Therefore, if we are not interested with this subtlety I'm fine with the .iszerodim option.

Regarding the .dim attribute, I think it is still relevant without Hilbert series since we already have a dimension function written by @RafaelDavidMohr.

[ederc]

What's about keeping .dim but adding a boolean field .dim_is_known? If this field is true, .dim can be used, otherwise values of .dim should be not used resp. meaningless.

[rprebet]

After discussing with @mohabsafey, I chose to initialize dim to nothing.

Then, when dimension isn't known, isnothing(I.dim) is true.
Else, in _core_msolve, if applicable, we set it to -1 or 0 (note that msolve always return 0 in jl_dim, even in the empty case).
In dimension, we set I.dim if it's not known (nothing).

I also add some consistency tests.

If you're ok, I'm fine with merging this PR.

[mohabsafey]

Many thanks. This is fine for me. If this is ok for @ederc, I merge it.

[ederc]

Fine for me.