Skip to content

Git issues 3937 1935 #3942

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

mikestillman
Copy link
Member

This PR should fix issues #3937, #1935. It removes the crash when groebner bases are given a Hilbert function "hint", and the data is not correct (so likely gives errors, but still, it should not crash).

Additionally:

  • A function canUseHilbertHint has been added, documented, which allows one (and the GB code) to decide if in a given situation, the Hilbert hint is usable.
  • Tests have been added
  • The abort when discovering a bad Hilbert function is removed. Instead, an exception is issued.
  • Note: multi-graded rings will not use any Hilbert hint. If desired, we can likely add this, but it will take some effort and time. Singly graded rings work, as long as all of the variables have positive degree.

@mikestillman mikestillman changed the base branch from stable to development July 30, 2025 01:38
exponents_t exp = newarray_atomic(int, P->n_vars());
exponents_t exp = new int[P->n_vars()];
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason behind this change? If this is correct in this case I wonder about tons of other exponents_t initializations which use newarray_atomic (only three exceptions, and this makes it four). The comment before newarray_atomic in newdelete.hpp says:

// this replaces all uses of the construction "new T[n]", with T containing NO pointers.

(canUseHilbertHint, Module)
(canUseHilbertHint, Matrix)
Headline
whether certain Groebner computations can make use of the Hilbert function
Copy link
Member

@mahrud mahrud Jul 30, 2025

Choose a reason for hiding this comment

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

I suspect nobody will actually find this node. How about this: add the example of passing the Hilbert option to one of the many nodes about Grobner bases instead, or add a separate node for the option [gb, Hilbert] and link to it from the documentation of gb. I would imagine canUseHilbertHint can stay internal (except imported when needed by MinimalPrimes) in that case.

@d-torrance
Copy link
Member

Should checkHilbertHint be exported? We're getting this error:

ii22 : assert checkHilbertHint generators graphIdeal f
../../../../../Macaulay2/tests/normal/ker.m2:24:23:(3):[5]: error: no method for adjacent objects:
            checkHilbertHint (of class Symbol)
    SPACE   | -p_0^4+p_1 -p_0^5+p_2 -p_0^6+p_3 | (of class Matrix)

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.

3 participants