Skip to content

Prevent race condition in makeMonomialOrdering#4299

Closed
d-torrance wants to merge 1 commit into
Macaulay2:developmentfrom
d-torrance:numerical-implicitization
Closed

Prevent race condition in makeMonomialOrdering#4299
d-torrance wants to merge 1 commit into
Macaulay2:developmentfrom
d-torrance:numerical-implicitization

Conversation

@d-torrance

Copy link
Copy Markdown
Member

A number of the variables used in this function are global, so we lock while modifying them so we can construct monomial orders in parallel.

This also gives us a chance to use the new Mutex class!

Before

i1 : unique \\ class \ taskResult \ apply(20, i -> schedule(() -> ZZ[x]))
/usr/share/Macaulay2/Core/engine.m2:103:47:(1):[18]: error: Inverses => true not compatible with GRevLex
/usr/share/Macaulay2/Core/engine.m2:113:15:(1):[17]: --back trace--
/usr/share/Macaulay2/Core/engine.m2:152:45:(1):[16]: --back trace--
/usr/share/Macaulay2/Core/engine.m2:211:51:(1):[14]: --back trace--
/usr/share/Macaulay2/Core/monoids.m2:565:61:(1):[13]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:130:48:(1):[12]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:146:20:(1):[11]: --back trace--
/usr/share/Macaulay2/Core/option.m2:17:14:(1):[10]: --back trace--
/usr/share/Macaulay2/Core/monoids.m2:589:17:(1):[6]: error: degree list should be of length 0
/usr/share/Macaulay2/Core/remember.m2:11:24:(1):[8]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:130:48:(1):[5]: --back trace--
/usr/share/Macaulay2/Core/polyrings.m2:90:31:(1):[7]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:146:20:(1):[4]: --back trace--
/usr/share/Macaulay2/Core/remember.m2:11:24:(1):[7]: --back trace--
/usr/share/Macaulay2/Core/polyrings.m2:138:55:(1):[2]: --back trace--
/usr/share/Macaulay2/Core/monoids.m2:587:62:(1):[6]: --back trace--
/usr/share/Macaulay2/Core/monoids.m2:589:17:(1):[6]: error: degree list should be of length 0
/usr/share/Macaulay2/Core/methods.m2:130:48:(1):[5]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:130:48:(1):[5]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:146:20:(1):[4]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:146:20:(1):/usr/share/Macaulay2/Core/polyrings.m2:138:55:(1):[2]: --back trace--
[4]: --back trace--
/usr/share/Macaulay2/Core/polyrings.m2:138:55:(1):[2]: --back trace--
/usr/share/Macaulay2/Core/monoids.m2:589:17:(1):[6]: error: degree list should be of length 0
/usr/share/Macaulay2/Core/methods.m2:130:48:(1):[5]: --back trace--
/usr/share/Macaulay2/Core/methods.m2:146:20:(1):[4]: --back trace--
/usr/share/Macaulay2/Core/polyrings.m2:138:55:(1):[2]: --back trace--

o1 = {Nothing, PolynomialRing}

o1 : List

After

i1 : unique \\ class \ taskResult \ apply(20, i -> schedule(() -> ZZ[x]))

o1 = {PolynomialRing}

o1 : List

Closes: #3239 (one of our greatest hits of GitHub workflow build failures!)

AI Disclosure

Claude Code zeroed in on what was causing check(6, "NumericalImplicitization") to fail frequently and wrote an early draft of this commit.

A number of the variables used in this function are global, so we
lock while modifying them so we can construct monomial orders in
parallel.
@mahrud

mahrud commented May 12, 2026

Copy link
Copy Markdown
Member

I was planning on using this as an exercise in debugging :)
That whole section of code should be refactored to be re-entrant IMO.

Comment thread M2/Macaulay2/m2/engine.m2
Comment on lines -217 to +220
(t,t',value logmo, logmo))
ret := (t,t',value logmo, logmo);
unlock makeMonomialOrderingMutex;
ret)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On my system, I've added a handy unlock(Mutex, Thing) := (M, x) -> ( unlock M; x ), so I can write:

     unlock(makeMonomialOrderingMutex, (t,t',value logmo, logmo)))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ooh, that's slick! Want to submit a PR to add it for everyone?

@d-torrance d-torrance added the threads Macaulay2/system label May 20, 2026
Comment thread M2/Macaulay2/m2/engine.m2
-- If it's not a list, we'll make a list of one element from it.
if monsize === null then monsize = null;
if not isListOfIntegers degs then error "expected a list of integers";
lock makeMonomialOrderingMutex;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we add a keyword of the form

withLock lock code

The idea here is you would do something like the following

withLock makeMonomialOrderingMutex (
    ordering = {MonomialSize => monsize, ordering};
     invert = inverses;
     ....
)

And it would lock the mutex before the block and unlock the mutex no matter how we exit the block, otherwise we have to be careful about exceptions or returns.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Code object in the interpreter is unique. Why do we need to specify the additional label makeMonomialOrderingMutex here?

@d-torrance

Copy link
Copy Markdown
Member Author

Closing in favor of #4367

@d-torrance d-torrance closed this May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

threads Macaulay2/system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants