[FTheoryTools] More on serialization of FTheoryTools

[HereAround]

As mentioned in the other PRs, it seemed natural to group all the different serialization discussions we had in one PR. For this, I need your help @antonydellavecchia whenever you are ready (not urgent).

Points to be addressed in this PR include (but are not necessarily limited to the following):

• The coordinates of the base space and ambient space should be remembered upon serialization. I will try to add a first step towards this goal in this PR/draft. As I see it, this requires that we add a list of strings (or symbols?) to the serialization of the models. An update of the QSM model artifacts is then needed.

The below points were initially meant to be fixed in this PR, but we (@antonydellavecchia and I) decided that it will be better to deal with them in separate PRs.

• Along these lines, we could also consider serialzing G4-fluxes (a.k.a. cohomology classes on toric spaces). At least for the QSMs, there is each time a specifically chosen G4-fluxes (cf. https://arxiv.org/pdf/2102.10115, equ. 2.25).
• Maybe wait for Serialization of Lie algebras #3980 and include the Lie algebras of the FTheoryModels in the serialization. In particular, this should be also applied to the QSM models.
• Add an artifact for the model discussed in [FTheoryTools] Add model 1511.03209 #3978. I have the corresponding .mrdi files and will be happy to share e.g. in slack or email. (This cannot be added here as github does not seem to support .mrdi files yet. And then, the files are about 0.5GB in size. So maybe best not to add them here.)
• Make use of the attribute saver that @antonydellavecchia is working on (cf. Serialize Types with Attributes #4020). For instance, currently the known resolutions are not saved for FTheory models.

cc @antonydellavecchia @emikelsons @apturner

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.58%. Comparing base (1217ab3) to head (0de86dc).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4008      +/-   ##
==========================================
- Coverage   84.60%   84.58%   -0.02%     
==========================================
  Files         631      631              
  Lines       84811    84830      +19     
==========================================
+ Hits        71751    71755       +4     
- Misses      13060    13075      +15     

Files with missing lines	Coverage Δ
...heoryTools/src/AbstractFTheoryModels/attributes.jl	99.59% <ø> (ø)
...TheoryTools/src/HypersurfaceModels/constructors.jl	96.72% <100.00%> (ø)
...eoryTools/src/Serialization/hypersurface_models.jl	100.00% <100.00%> (ø)
...ental/FTheoryTools/src/Serialization/qsm_models.jl	50.00% <ø> (-4.39%)	⬇️
...ntal/FTheoryTools/src/Serialization/tate_models.jl	97.50% <100.00%> (+0.06%)	⬆️
...heoryTools/src/Serialization/weierstrass_models.jl	97.50% <100.00%> (+0.06%)	⬆️
.../ToricVarieties/NormalToricVarieties/attributes.jl	98.66% <100.00%> (+0.01%)	⬆️
src/Serialization/ToricGeometry.jl	98.03% <ø> (-1.97%)	⬇️

... and 12 files with indirect coverage changes

[HereAround]

@antonydellavecchia , as we discussed during the coding sprint in KL, I will move most of the above ToDos to #2699 and will then try to brush up this PR.

[HereAround]

@antonydellavecchia I have brushed up this PR. In particular, I have updated the QSMDB. I am not sure if we can modify the current QSMDB artifact (attached to a release tag of OSCAR) I have moved it to https://martinbies.github.io/Materials/QSMDB/qsmdb.tar.gz, where I can modify/edit it more conveniently. Not sure if this is the best move though and am happy to change this. Please let me know what you think.

In any case - I would expect that with the current changes, all tests pass and thus, this PR is getting ready to be merged.

[HereAround]

@antonydellavecchia I am puzzled why the tests fail. The new artifact files should not cause the problem in the failures. So I am wondering if somehow OSCAR fails to fetch those new artifact files?

[HereAround]

@antonydellavecchia As discussed in slack, I have added the new files to the release tag now. Hopefully the tests do succeed now.

[HereAround]

Ok. I think I have finally succeeded. At least on my local system it works.

I have renamed the previous QSMDB into QSMDB-old and added the new version as QSMDB. The latter is larger in size by about 1MB than the old file. I believe this is because the hypersurface models do remember more details now than they used to at the time when the initial database was created.

[benlorenz]

Ok. I think I have finally succeeded. At least on my local system it works.

I have renamed the previous QSMDB into QSMDB-old and added the new version as QSMDB. The latter is larger in size by about 1MB than the old file. I believe this is because the hypersurface models do remember more details now than they used to at the time when the initial database was created.

I changed the name back (of the file in the github release), the old file must stay exactly as it is. It is still used in the existing oscar releases.
I moved the new file to QSMDB-2.tar.gz.

[benlorenz]

The name of the artifact and the filename in the url do not need to match, with the files renamed again as mentioned above the following works fine:

lorenz@eddie /tmp/artifacttest $ cat Artifacts.toml 
[QSMDB]
git-tree-sha1 = "ee9e07704b9e42ddd8f1d8751873f9e12a133318"
lazy = true

     [[QSMDB.download]]
     sha256 = "f2ce521a152738af28e957502a8fad3e84d9962937490e080a9b4bd598a8c191"
     url = "https://github.com/oscar-system/Oscar.jl/releases/download/archive-tag-1/QSMDB-2.tar.gz"     

lorenz@eddie /tmp/artifacttest $ julia-latest 
[...]
julia> using LazyArtifacts

julia> LazyArtifacts.ensure_artifact_installed("QSMDB", LazyArtifacts.find_artifacts_toml("."))
     Failure artifact: QSMDB
  Downloaded artifact: QSMDB
"/home/lorenz/.julia/artifacts/ee9e07704b9e42ddd8f1d8751873f9e12a133318"

shell> ls /home/lorenz/.julia/artifacts/ee9e07704b9e42ddd8f1d8751873f9e12a133318
1014.mrdi  1212.mrdi  138.mrdi	 1647.mrdi  1982.mrdi  2236.mrdi  2482.mrdi  2744.mrdi	2939.mrdi  3138.mrdi  3315.mrdi  3520.mrdi  377.mrdi   3965.mrdi  4175.mrdi  535.mrdi  772.mrdi
1015.mrdi  121.mrdi   139.mrdi	 1650.mrdi  1983.mrdi  2245.mrdi  2483.mrdi  274.mrdi	2942.mrdi  3140.mrdi  3326.mrdi  3521.mrdi  3782.mrdi  3967.mrdi  417.mrdi   557.mrdi  77.mrdi
...

Edit: That one download failure is probably because Pkg will try the package servers first and then use the specified url.

[HereAround]

Thank you @benlorenz ! I believe this PR is now ready to be reviewed.

[lgoettgens]

There are failures in the doctests

[HereAround]

There are failures in the doctests

Yeah, @benlorenz already commented on them above:

"That one download failure is probably because Pkg will try the package servers first and then use the specified url."

I do not know (yet) how to avoid this. Suggestions welcome.

Hhhmm... just realized that there are other failures too...

[benlorenz]

The failure I was referring to was just this line in my small example:

     Failure artifact: QSMDB

But this is resolved automatically by retrying:

  Downloaded artifact: QSMDB

[benlorenz]

The map creation (for map_from_torusinvariant_weil_divisor_group_to_class_group) needs to make sure that the codomain is correct for the case that the class group attribute was loaded from a file and thus exists without the map.

I pushed a fix to this branch which appends another (identity) map to the correct class group in that case:
https://github.com/HereAround/Oscar.jl/blob/6604d6e5c17f656b3b7589236ff5d8594fc37051/src/AlgebraicGeometry/ToricVarieties/NormalToricVarieties/attributes.jl#L752-L759

[HereAround]

The map creation (for map_from_torusinvariant_weil_divisor_group_to_class_group) needs to make sure that the codomain is correct for the case that the class group attribute was loaded from a file and thus exists without the map.

I pushed a fix to this branch which appends another (identity) map to the correct class group in that case: https://github.com/HereAround/Oscar.jl/blob/6604d6e5c17f656b3b7589236ff5d8594fc37051/src/AlgebraicGeometry/ToricVarieties/NormalToricVarieties/attributes.jl#L752-L759

Awesome! Very nice fix. Thank you very much indeed @benlorenz ! :)

[HereAround]

(Nitpick: I put the fix to class group as the first commit: In this way, I do expect that all commits work individually, and so the changes in this PR can be rebased onto the master branch, rather than squashed.)