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

Replace intarray with gc_vector<int>, combine varpower types + range-based loops #2770

Merged
merged 5 commits into from
Mar 13, 2023

Conversation

mahrud
Copy link
Member

@mahrud mahrud commented Feb 7, 2023

This commit replaces the type intarray throughout the engine with a specialization of std::vector which uses the GC allocator:

  • replaced intarray with gc_vector

Note: gc_vector<T> is basically VECTOR(T), so at some point I'll also replace all VECTOR(T) with the new gc_vector<T> syntax.

This commit replaces the varpower types into a templated type that uses gc_vector<int>:

  • combined varpower types into ExponentList

The last two commits are pretty minor changes:

  • switched to range-based loops for Nterms
  • renamed and moved IM2_kernel_of_GB -> rawKernelOfGB

@mikestillman
Copy link
Member

For the replacement name for varpower monomials, how about SparseExponents or ExponentMap? The main problem with this last name convention is that in the noncommutative case, it is not a map at all... I guess ExponentList is perhaps more correct, but doesn't necessarily convey any other information (e.g. that it is only representing those exponents which are non-zero).

@mikestillman
Copy link
Member

@mahrud I'm going to start reviewing this so we will be ready once we choose the replacement name for varpower monomials

@mahrud
Copy link
Member Author

mahrud commented Feb 9, 2023

@mahrud I'm going to start reviewing this so we will be ready once we choose the replacement name for varpower monomials

It might be easier to accept #2769 first, which currently contributes to a big chunk of the diff in this PR.

@mikestillman
Copy link
Member

@mahrud Thanks for the heads up, I mistook this one for the next one in the series of pull requests. I'll review #2769 first.

@mikestillman
Copy link
Member

@mahrud It looks like there are many conflicts now that I rebased and merged #2769 . After you take a look at these, I'll start reviewing this one.

@mahrud
Copy link
Member Author

mahrud commented Feb 10, 2023

@mahrud It looks like there are many conflicts now that I rebased and merged #2769 . After you take a look at these, I'll start reviewing this one.

Done!

@mahrud mahrud marked this pull request as ready for review February 10, 2023 14:45
Copy link
Member

@mikestillman mikestillman left a comment

Choose a reason for hiding this comment

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

Hi @mahrud, thanks for making these changes! I have flagged a few places where there might be an issue. Once you look at those, I'll accept this pull request. I'd also like @antonleykin to see how his NAG code had changed, so it won't come as a surprise later!

M2/Macaulay2/e/debug.cpp Show resolved Hide resolved
M2/Macaulay2/e/f4/varpower-monomial.hpp Show resolved Hide resolved
M2/Macaulay2/e/hilb.cpp Show resolved Hide resolved
M2/Macaulay2/e/interface/factory.cpp Show resolved Hide resolved
M2/Macaulay2/e/interface/factory.cpp Show resolved Hide resolved
M2/Macaulay2/e/schorder.cpp Show resolved Hide resolved
M2/Macaulay2/e/ExponentList.hpp Show resolved Hide resolved
M2/Macaulay2/e/NAG.cpp Show resolved Hide resolved
M2/Macaulay2/e/poly.cpp Show resolved Hide resolved
M2/Macaulay2/e/poly.cpp Show resolved Hide resolved
@mikestillman mikestillman merged commit 6968ab3 into Macaulay2:development Mar 13, 2023
@mahrud mahrud deleted the feature/gc_vector branch March 13, 2023 18:11
@mahrud mahrud mentioned this pull request Mar 13, 2023
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