Sitelet https://github.com/math-comp/math-comp/pull/1435
Skip to content

Implemented wrapping of mixin in bigop.v for files monoid.v and nmodule.v - #1435

Draft
CalosciMatteo wants to merge 77 commits into
math-comp:masterfrom
CalosciMatteo:algebrawithwrap
Draft

CalosciMatteo wants to merge 77 commits into
math-comp:masterfrom
CalosciMatteo:algebrawithwrap

Conversation

@CalosciMatteo

@CalosciMatteo CalosciMatteo commented May 19, 2025 •

Copy link
Copy Markdown

make now crash at ssralg.v, this need to be inspected.

This is intended to compile with HB#wrapping.

  • In bigop.v added the structure Semigroup.Com of commutative operations and the structure Monoid.PreLaw of unital operations

  • In monoid.v wrapped the associativity mixin for operation SemiGroup.isLaw explictly in the wrapper SemiGroupisLaw__on__Magma_mul. The old mixin Magma_isSemigroup is now a factory

  • In monoid.v wrapped the unital mixin for operation Monoid.isMonoidLaw explictly in the wrapper isMonoidLaw__on__BaseUMagma_MulOne. The old mixin BaseUMagma_isUMagma is now a factory

  • In nmodule.v wrapped the commutativity mixin for operation SemiGroup.isCommutativeLaw explicitly in the wrapper SemiGroupisCommutativeLaw__on__BaseAddMagma_add. The old mixin BaseAddMagma_isAddMagma is now a factory

  • In nmodule.v wrapped the associativity mixin SemiGroup.isLaw explicitly in the wrapper SemiGroupisLaw__on__BaseAddMagma_add. The old mixin AddMagma_isAddSemigroup is now a factory

  • In Monoid.isMonoidLaw wrapped the unital mixin for operation Monoid.isMonoidLaw explicitly in the wrapper MonoidisMonoidLaw__on__BaseAddUMagma_addZero. The old mixin BaseAddUMagma_isAddUMagma is now a factory

  • In wrapped the mixin explicitly in the wrapper . The old mixin is now a factory

  • Explicitly added some canonical projections not correctly generated by HB (marked as BUG in the comment)

  • Some comments marked TODO

  • Exposed some HB bug, some already have fixes ready to test

Motivation for this change
Minimal TODO list
  • added changelog entries with doc/changelog/make-entry.sh
  • added corresponding documentation in the headers
  • tried to abide by the contribution guide
  • this PR contains an optimum number of meaningful commits

See this Checklist for details.

Automatic note to reviewers

Read this Checklist.

…nmodule.v`.

`make` now crash at ssralg.v, this need to be inspected.

This is intended to compile with HB#wrapping at commit 671d4f747141c2ee2bb5409f38bca20f18dde6a5

- In `bigop.v` added the structure `Semigroup.Com` of commutative operations and the structure `Monoid.PreLaw` of unital operations

- In `monoid.v` wrapped the associativity mixin for operation `SemiGroup.isLaw` explictly in the wrapper `SemiGroupisLaw__on__Magma_mul`. The old mixin `Magma_isSemigroup` is now a factory
- In `monoid.v` wrapped the unital mixin for operation `Monoid.isMonoidLaw` explictly in the wrapper `isMonoidLaw__on__BaseUMagma_MulOne`. The old mixin `BaseUMagma_isUMagma` is now a factory

- In `nmodule.v` wrapped the commutativity mixin for operation `SemiGroup.isCommutativeLaw ` explicitly in the wrapper `SemiGroupisCommutativeLaw__on__BaseAddMagma_add`. The old mixin `BaseAddMagma_isAddMagma` is now a factory
- In `nmodule.v` wrapped the associativity mixin `SemiGroup.isLaw` explicitly in the wrapper `SemiGroupisLaw__on__BaseAddMagma_add`. The old mixin `AddMagma_isAddSemigroup` is now a factory
- In `Monoid.isMonoidLaw` wrapped the unital mixin for operation `Monoid.isMonoidLaw` explicitly in the wrapper `MonoidisMonoidLaw__on__BaseAddUMagma_addZero`. The old mixin `BaseAddUMagma_isAddUMagma` is now a factory
- In `` wrapped the mixin `` explicitly in the wrapper ``. The old mixin `` is now a factory

- Explicitly added some canonical projections not correctly generated by HB (marked as BUG in the comment)
- Some comments marked TODO
- Exposed some HB bug, some already have fixes ready to test
@pi8027 pi8027 changed the title Implemented wrapping of mixin in bigop.v for files monoid.v and `… Implemented wrapping of mixin in bigop.v for files monoid.v and nmodule.v May 20, 2025
CalosciMatteo and others added 28 commits May 20, 2025 17:55
In some cases, as in `hierarchy-builder#wrapping` `tests/MinimalWrapBugs/canonicalByHand.v`, some canonical projections does not get generated automatically. After cb1a6ec68e14cb325c7116da4995451fb929fbec they can be generated by calling `HB.saturate(@key _)`.

This commit applies such fix to the identified instances in `monoid.v` and `nmodule.v`.

Also, modified comment exposing a bug in `nmodule.v`
- some comments with renaming to be considered in `bigop.v`

- In `nmodule.v` added a structure with just the mixin `hasZero`. This is necessary to avoid an error in `ssralg.v`

- In `nmodule.v` adjusted a comment related to a bug in instantiating commutativity for the key `to_multiplicative` (the whole piece of code seems to be superfluos at this point)

- In `ssralg.v` imported `monoid.v`, so that the mixin of `bigop.v` can be used for the moltiplicative part of ring/algebras

- In `ssralg.v` wrapped the distributivity and annihilative mixins in explicit wrappers. The old mixin `Nmodule_isPzSemiRing` is now a factory.
	(in the process defining extra structures is necessary to avoid the bug exposed in HB#wrapping tests/MinimalWrapBugs/structVS2mixin.v*)

- Applied workaround (denoted by comments) for a bug not allowing to use some factories (involving wrapped mixin) to instantiate structures

NOTE: somehow something is changed in the way the tactic `rewrite` match the goals. This required some minor adjustment to the proof of `exprMn_n`, `prodrN`, `sqrrB1`. Also a type in `exp` needs to be explictly specified (all in `ssralg.v`)

NOTE: `ssralg.v` now crashes after the `Scale` module.
Added comment suggesting a renaming
In c7857f24eaa4149d9021955666b48eb4ef346c69 an HB bug was fixed concerning the declaration of instances using factories involving wrapped arguments.

This commit replace the workarounds for such bug with `HB.instance Definition _ := [factory name].Build ...`
Following the fix in c7857f24eaa4149d9021955666b48eb4ef346c69 we can instantiate without wrapping a key which was wrapped before
I suggest to consider deprecating it since its just a duplicate of `isUMagmaMorphism`
Is it ok to export all the structures this way?

Something seems off with `rewrite`: it search first in the RHS. Similiar problem appear once when  introducing variables

The fail in `numdomain.v` seems to be related to scopes.
CalosciMatteo and others added 23 commits July 1, 2025 15:16
`mulrn_char` is never used as a notation in this file and is given as a definition later in the file. Leaving both yelds `Error: Attribute for note specified twice.`
…tive one.

We thus make it local and use an alias in order to still have a global one
@coqbot-app coqbot-app Bot removed the needs: rebase PR which is not rebased: check the target is appropriate (generally master) and rebase on top of it. label Nov 26, 2025
CalosciMatteo and others added 6 commits December 3, 2025 19:04
fixed by setting primitive projections for wrapper record.
…tructure in fingroupmixin and the intended ring structure.

Here, since group is under ring, we have the multiplicative structure there (which is not a group since there are not inverses).

We work around this by juggling aliases. Maybe something like `finzmod` would be needed as a long term fix

This branch has not been deployed

No deployments
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.

6 participants