Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hasse-Schmidt derivatives 2.0 #4272

Merged
merged 12 commits into from
Nov 15, 2024

Conversation

KilianBruns
Copy link
Contributor

@KilianBruns KilianBruns commented Nov 5, 2024

This is a new version of the abbandoned pull request #3912
Most comments of the last pull request have been considered.

Added

  • Hasse-Schmidt derivatives for polynomials

(Implemented at the request of @afkafkafk13)

Closes #3912 .

Copy link
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments about the bibliography. I am not familiar with the mathematics and will leave that for Anne to comment on.

docs/oscar_references.bib Outdated Show resolved Hide resolved
docs/oscar_references.bib Outdated Show resolved Hide resolved
experimental/HasseSchmidt/src/HasseSchmidt.jl Outdated Show resolved Hide resolved
docs/oscar_references.bib Outdated Show resolved Hide resolved
@KilianBruns KilianBruns marked this pull request as draft November 6, 2024 09:13
Copy link

codecov bot commented Nov 7, 2024

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.

Project coverage is 84.52%. Comparing base (aea00d0) to head (6c93d74).
Report is 38 commits behind head on master.

Files with missing lines Patch % Lines
experimental/HasseSchmidt/src/HasseSchmidt.jl 64.70% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4272      +/-   ##
==========================================
+ Coverage   84.48%   84.52%   +0.04%     
==========================================
  Files         641      646       +5     
  Lines       85427    85606     +179     
==========================================
+ Hits        72170    72359     +189     
+ Misses      13257    13247      -10     
Files with missing lines Coverage Δ
experimental/HasseSchmidt/src/HasseSchmidt.jl 64.70% <64.70%> (ø)

... and 54 files with indirect coverage changes

@afkafkafk13
Copy link
Collaborator

Algorithm and documentation look good now.
Did you already run bibtool to make changes such that everything complies to the Oscar-bibliography rules?

@fingolfin
Copy link
Member

@KilianBruns I don't understand, why is #3912 "abandoned", aren't you the author as well??

But if you really want a new branch and PR, please close the old one.

@joschmitt
Copy link
Member

@KilianBruns I just cleaned up the bibliography using bibtool and added some information from mathscinet. You seem to have added a paper by Hazewinkel both in the arxiv and the published version, I assume we can remove the arxiv one? Also, you added a paper by Hasse which you don't reference from anywhere. Or did I miss this?

Copy link
Collaborator

@afkafkafk13 afkafkafk13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to go, as soon as the two remaining points (deleting a reference and two lines) have been treated.

@KilianBruns Thank you for doing this PR
@joschmitt Thank you for cleaning up the bibliography

docs/oscar_references.bib Outdated Show resolved Hide resolved
experimental/HasseSchmidt/src/HasseSchmidt.jl Outdated Show resolved Hide resolved
@KilianBruns
Copy link
Contributor Author

Thank you all for your help!

@lgoettgens lgoettgens added the experimental Only changes experimental parts of the code label Nov 15, 2024
@afkafkafk13 afkafkafk13 marked this pull request as ready for review November 15, 2024 22:04
@afkafkafk13 afkafkafk13 merged commit 99f0f7b into oscar-system:master Nov 15, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Only changes experimental parts of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants