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

add package NumericalSemigroups #3540

Merged
merged 20 commits into from
Nov 1, 2024
Merged

Conversation

eisenbud
Copy link
Contributor

New package created by Frank Schreyer and David Eisenbud with basic commands for analyzing semigroups using
the Kunz cones
and studying when they are Weierstrass semigroups.

@eisenbud
Copy link
Contributor Author

Also updated "PencilsOfQuadrics"

@eisenbud
Copy link
Contributor Author

many tests in Pencil... used random numbers and thus failed randomly. I've inserted setRandomSeed commands to fix this.

@d-torrance
Copy link
Member

The random seed is always set to 0 by default for tests and examples, so I don't think the most recent commit is necessary.

The failing build from earlier was unrelated to this PR (#1579).

@eisenbud
Copy link
Contributor Author

Thanks, Doug -- This is my first experience(s) doing a pull request by myself. I had assumed that the failing sumsOfSquares call indicated in the "Details" was something indirectly called from my package. Should I have just ignored the failure?
-David

@d-torrance
Copy link
Member

d-torrance commented Oct 26, 2024

Often build failures are related to the PR (like the Homepage one earlier), so I wouldn't say you should ignore them in general.

If you're not sure, then you can search through the existing issues to see if the failure is a known problem like this one, or just ask!

@mahrud
Copy link
Member

mahrud commented Oct 26, 2024

Usually if only a single build fails it could be one of these unrelated but persistent failures that show up randomly, but if two fail probably one of the tests failed and four fail it's probably a failing example.

Perhaps at some point we can have GitHub annotations #3533 to easily tell us what failed without having to dig through the build log.

@d-torrance
Copy link
Member

@eisenbud - Did you want to keep the setRandomSeed commit? If not, you can run the following:

git reset --hard HEAD~
git push --force <remote> development

HEAD~ refers to the commit immediately preceding the current one. Replace <remote> with whatever name you've given your fork.

@mikestillman
Copy link
Member

@d-torrance @eisenbud The one failing test (I think) is the SumOfSquares failure, which should not be related to this PR.
Our Complexes PR (#3526) will make changes to PencilOfQuadrics too. This PR should be done before #3526, that way, I can deal with the resulting conflicts.

@d-torrance
Copy link
Member

The one failing test (I think) is the SumOfSquares failure, which should not be related to this PR.

That's correct -- see #1579. @mahrud has a fix in #3519

I'm not sure if this should be merged now or not. The final commit (3e672a6) may not be necessary.

@mahrud
Copy link
Member

mahrud commented Oct 30, 2024

David, do you mind if I force push a changes to your branch to remove the last commit and fix the failing build?

@mahrud mahrud added this to the version 1.24.11 milestone Oct 30, 2024
--test of preRandomLineBundle and randomLineBundle
restart
load "PencilsOfQuadrics.m2"
>>>>>>> 82c56807936dda91b86f9ccae7189b001e15ff19
Copy link
Member

Choose a reason for hiding this comment

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

This line is going to cause errors (and also the "restart" two lines above will prevent this test from running)

@d-torrance
Copy link
Member

The build failure is unrelated (#3239). Merging!

@d-torrance d-torrance merged commit d1c5ca8 into Macaulay2:development Nov 1, 2024
4 of 5 checks passed
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.

4 participants