Add documentation for action polynomial rings#5329
Add documentation for action polynomial rings#5329lgoettgens merged 21 commits intooscar-system:masterfrom
Conversation
lgoettgens
added
experimental
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive documentation for the experimental action polynomial rings functionality that was introduced in #5212. It includes markdown documentation files covering construction, usage patterns, and theoretical background. Additionally, it includes several bug fixes and code improvements to streamline the API.
- Added documentation files for action polynomial rings, difference/differential polynomial rings, and rankings
- Renamed
ndiffston_action_mapsthroughout the codebase for better clarity - Fixed inconsistent variable naming and improved code documentation
- Enhanced test coverage with additional permutation tests
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| experimental/ActionPolyRing/test/ActionPolyRing.jl | Updated tests to use renamed n_action_maps function and added permutation tests |
| experimental/ActionPolyRing/src/exports.jl | Updated export to use n_action_maps instead of ndiffs |
| experimental/ActionPolyRing/src/Types.jl | Renamed field and parameters from ndiffs to n_action_maps, improved variable naming |
| experimental/ActionPolyRing/src/IO.jl | Updated display methods to use renamed function and fixed variable references |
| experimental/ActionPolyRing/src/Content.jl | Comprehensive updates to use n_action_maps, improved documentation, and enhanced function signatures |
| experimental/ActionPolyRing/docs/src/*.md | New documentation files covering action polynomial rings functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I mis-clicked and asked copilot for a review. I don't know how good the quality of this review is, but maybe it finds some typos. @SirToby25 feel free to ignore any of its comments (aka mark them as resolved) |
… into tb/ActionPolyRingDocs
|
Actually, the review catched some obvious mistakes. I am grateful for Copilot. |
Co-authored-by: Max Horn <max@quendi.de>
|
@lgoettgens do you have any further suggestions? |
lgoettgens
left a comment
lgoettgens
left a comment
There was a problem hiding this comment.
some more comments, mostly about grammar. I didn't look at the mathematics, but I trust you to build the docs and verify all the mathjax yourself
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
… into tb/ActionPolyRingDocs
…gs.md Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
| @doc raw""" | ||
| is_univariate(p::ActionPolyRing) | ||
|
|
||
| Return `false`, since an action polynomial ring cannot be univariate. |
There was a problem hiding this comment.
Not that I mind, but why even define this if it is constant?
Of course is_univariate is part of the MPolyRing API, but ActionPolyRing is not a subtype of that and so is not obliged to provide this.
To be clear, I am not at all opposed to adding this if it seems useful to you, I just wanted to point out that you are not at all obliged to have this.
|
Just talked to @SirToby25 and we are proceeding with the merge (and thus ignoring #5329 (comment), at least for now) |
This PR contains the markdown files for the OSCAR documentation of the experimental action polynomial rings that were added in #5212 . It also contains some minor bug fixes and streamlined code documentation.