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

added compatibility with Flint 3.1.0 #3152

Merged

Conversation

mahrud
Copy link
Member

@mahrud mahrud commented Mar 5, 2024

No description provided.

@d-torrance
Copy link
Member

I'm still getting a build error with FLINT 3.1.0 after applying this patch:

CXX NCAlgebras/NCF4.cpp
In file included from /home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/flint.h:197,
                 from ../../../../Macaulay2/e/aring-qq-flint.hpp:13,
                 from ../../../../Macaulay2/e/aring-qq.hpp:6,
                 from ../../../../Macaulay2/e/aring-translate.hpp:18,
                 from ../../../../Macaulay2/e/aring-glue.hpp:7,
                 from ../../../../Macaulay2/e/VectorArithmetic.hpp:42,
                 from ../../../../Macaulay2/e/NCAlgebras/NCF4.hpp:14,
                 from ../../../../Macaulay2/e/NCAlgebras/NCF4.cpp:1:
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:74: warning: "__ll_B" redefined
   74 | #define __ll_B ((ulong) 1 << (FLINT_BITS / 2))
      | 
In file included from /usr/include/recint/recdefine.h:145,
                 from /usr/include/recint/ruruint.h:43,
                 from /usr/include/recint/rumanip.h:42,
                 from /usr/include/recint/ruconvert.h:21,
                 from /usr/include/gmp++/gmp++_int.h:26,
                 from /usr/include/gmp++/gmp++.h:57,
                 from /usr/include/givaro/givinteger.h:19,
                 from /usr/include/givaro/givpoly1dense.h:22,
                 from /usr/include/givaro/givpoly1.h:38,
                 from /usr/include/fflas-ffpack/ffpack/ffpack.h:38,
                 from ../../../../Macaulay2/e/aring-zzp-ffpack.hpp:17,
                 from ../../../../Macaulay2/e/aring-translate.hpp:17:
/usr/include/recint/reclonglong.h:46: note: this is the location of the previous definition
   46 | #define __ll_B ((UWtype) 1 << (W_TYPE_SIZE / 2))
      | 
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:75: warning: "__ll_lowpart" redefined
   75 | #define __ll_lowpart(t) ((ulong) (t) & (__ll_B - 1))
      | 
/usr/include/recint/reclonglong.h:47: note: this is the location of the previous definition
   47 | #define __ll_lowpart(t) ((UWtype) (t) & (__ll_B - 1))
      | 
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:76: warning: "__ll_highpart" redefined
   76 | #define __ll_highpart(t) ((ulong) (t) >> (FLINT_BITS / 2))
      | 
/usr/include/recint/reclonglong.h:48: note: this is the location of the previous definition
   48 | #define __ll_highpart(t) ((UWtype) (t) >> (W_TYPE_SIZE / 2))
      | 
In file included from ../../../../Macaulay2/e/aring-translate.hpp:24:
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::negate(ElementType&, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:121:14: error: ‘nmod_neg’ was not declared in this scope; did you mean ‘nmod_t’?
  121 |     result = nmod_neg(a, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::invert(ElementType&, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:128:14: error: ‘n_invmod’ was not declared in this scope; did you mean ‘fmpz_invmod’?
  128 |     result = n_invmod(a, mCharac);
      |              ^~~~~~~~
      |              fmpz_invmod
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::add(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:133:14: error: ‘nmod_add’ was not declared in this scope; did you mean ‘nmod_t’?
  133 |     result = nmod_add(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::subtract(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:138:14: error: ‘nmod_sub’ was not declared in this scope; did you mean ‘nmod_t’?
  138 |     result = nmod_sub(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::subtract_multiple(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:145:22: error: ‘nmod_neg’ was not declared in this scope; did you mean ‘nmod_t’?
  145 |     ElementType a1 = nmod_neg(a, mModulus);
      |                      ^~~~~~~~
      |                      nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp:160:5: error: ‘NMOD_ADDMUL’ was not declared in this scope
  160 |     NMOD_ADDMUL(result, a1, b, mModulus);
      |     ^~~~~~~~~~~
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::mult(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:165:14: error: ‘nmod_mul’ was not declared in this scope; did you mean ‘nmod_t’?
  165 |     result = nmod_mul(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::divide(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:172:14: error: ‘nmod_div’ was not declared in this scope; did you mean ‘nmod_t’?
  172 |     result = nmod_div(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::power(ElementType&, ElementType, long int) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:179:18: error: ‘n_powmod2_preinv’ was not declared in this scope
  179 |         result = n_powmod2_preinv(a, n, mModulus.n, mModulus.ninv);
      |                  ^~~~~~~~~~~~~~~~
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::power_mpz(ElementType&, ElementType, mpz_srcptr) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:195:18: error: ‘n_powmod2_ui_preinv’ was not declared in this scope
  195 |         result = n_powmod2_ui_preinv(a, nbar, mModulus.n, mModulus.ninv);
      |                  ^~~~~~~~~~~~~~~~~~~
In file included from ../../../../Macaulay2/e/mutablemat-defs.hpp:86,
                 from ../../../../Macaulay2/e/mutablemat.hpp:6,
                 from ../../../../Macaulay2/e/aring-glue.hpp:10:
../../../../Macaulay2/e/mat-linalg.hpp: In function ‘size_t MatrixOps::rowReducedEchelonForm(const DMatGFFlint&, DMatGFFlint&)’:
../../../../Macaulay2/e/mat-linalg.hpp:932:71: error: cannot convert ‘const fq_zech_ctx_struct*’ to ‘const fq_zech_mat_struct*’
  932 | ong rank = fq_zech_mat_rref(A1.fq_zech_mat(), A.ring().flintContext());
      |                                               ~~~~~~~~~~~~~~~~~~~~~^~
      |                                                                    |
      |                                                                    const fq_zech_ctx_struct*

In file included from /home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/fq_zech_mat.h:36,
                 from ../../../../Macaulay2/e/dmat-gf-flint.hpp:15,
                 from ../../../../Macaulay2/e/dmat.hpp:69,
                 from ../../../../Macaulay2/e/mutablemat-defs.hpp:82:
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/fq_mat_templates.h:247:76: note:   initializing argument 2 of ‘mp_limb_signed_t fq_zech_mat_rref(fq_zech_mat_struct*, const fq_zech_mat_struct*, const fq_zech_ctx_struct*)’
  247 | T, mat_rref)(TEMPLATE(T, mat_t) B, const TEMPLATE(T, mat_t) A, const TEMPLATE(T, ctx_t) ctx);
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~^

At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-macro-redefined’ may have been intended to silence earlier diagnostics
make[2]: *** [Makefile.common:33: NCAlgebras/NCF4.o] Error 1
make[2]: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/Macaulay2/e'
make[1]: *** [Makefile:15: all-in-e] Error 2
make[1]: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/Macaulay2'
make: *** [GNUmakefile:251: all-in-Macaulay2] Error 2
make: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug'

@mahrud
Copy link
Member Author

mahrud commented Mar 5, 2024

Yea I realized it's not enough. I'm evaluating how twisted of a solution is warranted here (e.g. a macro that repeats a variable depending on version of flint, or just a bunch of if/elses)

@mahrud mahrud force-pushed the release-1.23-branch branch from 645ab54 to 4f4146b Compare March 5, 2024 00:48
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.

If this compiles under flint 3.1.0, it looks good (and a simple fix too).

@mikestillman
Copy link
Member

I was getting ready to look at this, but it looks like you have it done! Is this all set possibly? I'll look at the changes you made, actually, just did, looks good. It looks like it is not running builds though. @DanGrayson, should I just accept it?

@mahrud
Copy link
Member Author

mahrud commented Mar 5, 2024

This branch isn't set up to have tests. I don't really understand why there's a separate branch for the release. Everything should be happening on development and occasionally there should be a new release tag.

@mikestillman
Copy link
Member

@mahrud If I update brew now, it will install flint 3.1.0, and remove the old version? So if I want to keep doing development on other branches, I'll need to cherry pick these fixes in to these branches?

@mahrud
Copy link
Member Author

mahrud commented Mar 5, 2024

Not to mention that this commit is will be missing from development, so you'd need to merge this branch into development, adding an entirely unnecessary merge commit.

@d-torrance
Copy link
Member

I'm still getting an error:

make -C e all 
make[2]: Entering directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/Macaulay2/e'
CXX NCAlgebras/NCF4.cpp
In file included from /home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/flint.h:197,
                 from ../../../../Macaulay2/e/aring-qq-flint.hpp:13,
                 from ../../../../Macaulay2/e/aring-qq.hpp:6,
                 from ../../../../Macaulay2/e/aring-translate.hpp:18,
                 from ../../../../Macaulay2/e/aring-glue.hpp:7,
                 from ../../../../Macaulay2/e/VectorArithmetic.hpp:42,
                 from ../../../../Macaulay2/e/NCAlgebras/NCF4.hpp:14,
                 from ../../../../Macaulay2/e/NCAlgebras/NCF4.cpp:1:
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:74: warning: "__ll_B" redefined
   74 | #define __ll_B ((ulong) 1 << (FLINT_BITS / 2))
      | 
In file included from /usr/include/recint/recdefine.h:145,
                 from /usr/include/recint/ruruint.h:43,
                 from /usr/include/recint/rumanip.h:42,
                 from /usr/include/recint/ruconvert.h:21,
                 from /usr/include/gmp++/gmp++_int.h:26,
                 from /usr/include/gmp++/gmp++.h:57,
                 from /usr/include/givaro/givinteger.h:19,
                 from /usr/include/givaro/givpoly1dense.h:22,
                 from /usr/include/givaro/givpoly1.h:38,
                 from /usr/include/fflas-ffpack/ffpack/ffpack.h:38,
                 from ../../../../Macaulay2/e/aring-zzp-ffpack.hpp:17,
                 from ../../../../Macaulay2/e/aring-translate.hpp:17:
/usr/include/recint/reclonglong.h:46: note: this is the location of the previous definition
   46 | #define __ll_B ((UWtype) 1 << (W_TYPE_SIZE / 2))
      | 
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:75: warning: "__ll_lowpart" redefined
   75 | #define __ll_lowpart(t) ((ulong) (t) & (__ll_B - 1))
      | 
/usr/include/recint/reclonglong.h:47: note: this is the location of the previous definition
   47 | #define __ll_lowpart(t) ((UWtype) (t) & (__ll_B - 1))
      | 
/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/usr-host/include/flint/longlong.h:76: warning: "__ll_highpart" redefined
   76 | #define __ll_highpart(t) ((ulong) (t) >> (FLINT_BITS / 2))
      | 
/usr/include/recint/reclonglong.h:48: note: this is the location of the previous definition
   48 | #define __ll_highpart(t) ((UWtype) (t) >> (W_TYPE_SIZE / 2))
      | 
In file included from ../../../../Macaulay2/e/aring-translate.hpp:24:
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::negate(ElementType&, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:121:14: error: ‘nmod_neg’ was not declared in this scope; did you mean ‘nmod_t’?
  121 |     result = nmod_neg(a, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::invert(ElementType&, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:128:14: error: ‘n_invmod’ was not declared in this scope; did you mean ‘fmpz_invmod’?
  128 |     result = n_invmod(a, mCharac);
      |              ^~~~~~~~
      |              fmpz_invmod
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::add(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:133:14: error: ‘nmod_add’ was not declared in this scope; did you mean ‘nmod_t’?
  133 |     result = nmod_add(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::subtract(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:138:14: error: ‘nmod_sub’ was not declared in this scope; did you mean ‘nmod_t’?
  138 |     result = nmod_sub(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::subtract_multiple(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:145:22: error: ‘nmod_neg’ was not declared in this scope; did you mean ‘nmod_t’?
  145 |     ElementType a1 = nmod_neg(a, mModulus);
      |                      ^~~~~~~~
      |                      nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp:160:5: error: ‘NMOD_ADDMUL’ was not declared in this scope
  160 |     NMOD_ADDMUL(result, a1, b, mModulus);
      |     ^~~~~~~~~~~
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::mult(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:165:14: error: ‘nmod_mul’ was not declared in this scope; did you mean ‘nmod_t’?
  165 |     result = nmod_mul(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::divide(ElementType&, ElementType, ElementType) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:172:14: error: ‘nmod_div’ was not declared in this scope; did you mean ‘nmod_t’?
  172 |     result = nmod_div(a, b, mModulus);
      |              ^~~~~~~~
      |              nmod_t
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::power(ElementType&, ElementType, long int) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:179:18: error: ‘n_powmod2_preinv’ was not declared in this scope
  179 |         result = n_powmod2_preinv(a, n, mModulus.n, mModulus.ninv);
      |                  ^~~~~~~~~~~~~~~~
../../../../Macaulay2/e/aring-zzp-flint.hpp: In member function ‘void M2::ARingZZpFlint::power_mpz(ElementType&, ElementType, mpz_srcptr) const’:
../../../../Macaulay2/e/aring-zzp-flint.hpp:195:18: error: ‘n_powmod2_ui_preinv’ was not declared in this scope
  195 |         result = n_powmod2_ui_preinv(a, nbar, mModulus.n, mModulus.ninv);
      |                  ^~~~~~~~~~~~~~~~~~~
At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-macro-redefined’ may have been intended to silence earlier diagnostics
make[2]: *** [Makefile.common:33: NCAlgebras/NCF4.o] Error 1
make[2]: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/Macaulay2/e'
make[1]: *** [Makefile:15: all-in-e] Error 2
make[1]: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug/Macaulay2'
make: *** [GNUmakefile:251: all-in-Macaulay2] Error 2
make: Leaving directory '/home/profzoom/src/macaulay2/M2/M2/BUILD/doug'

@mahrud
Copy link
Member Author

mahrud commented Mar 5, 2024

The builds worked locally for me and also here on ubuntu and macOS: Macaulay2/homebrew-tap#217

@d-torrance
Copy link
Member

d-torrance commented Mar 5, 2024

The recint stuff makes me think it might be another givaro/flint header order thing...

@mikestillman
Copy link
Member

@d-torrance I'll take a look. Is this the only commit so far tying to fix flint 3.1.0 use?

@mahrud @DanGrayson @d-torrance Maybe we could streamline build release branches/tags. We should discuss what we can do to streamline the build process, especially if the two of you (or a subset, or others too) will be doing releases going forward.

@mikestillman
Copy link
Member

I think I'll plan on localizing all use of ffpack and givaro to one file (that doesn't have flint either). This will entail removing ARing types from FFPACK and Givaro (or rather, using them in one file only, for computing rank of a dense matrix over a prime finite field. This will not be for this release.

@mikestillman
Copy link
Member

mikestillman commented Mar 5, 2024

@d-torrance This appears in aring-zzp-flint.hpp. I don't see the define anywhere. Do you know how this is being set?

 #ifdef HAVE_FLINT_NMOD_H
  #include <flint/nmod.h>   // for nmod_neg, nmod_add, nmod_div, nmod_mul
#endif

edit: nevermind, I didn't grep far enough up the food chain, it os being set during configure time.

It certainly seems that on the machine where the failure is occuring, it is either not including this file, or if so, it isn't any longer referring to these functions?

@d-torrance
Copy link
Member

Yes -- it's set during configuration. See the following commits:

@d-torrance
Copy link
Member

Mike, I think you're right! The autotools build only sets that #define if it detects a newer version of FLINT during configuration. But I'm building FLINT after that.

@d-torrance
Copy link
Member

Yup, that did the trick! I'm still getting those recint warnings, but no more errors.

@d-torrance
Copy link
Member

The recint compiler warnings go away if the FLINT headers appear before the Givaro ones, but swapping the order leads to errors on various architectures, so I guess we'll just have to deal with them. I've submitted an issue to Givaro.

@mahrud
Copy link
Member Author

mahrud commented Mar 5, 2024

Maybe we could streamline build release branches/tags. We should discuss what we can do to streamline the build process, especially if the two of you (or a subset, or others too) will be doing releases going forward.

I didn't realize that was the plan. Is this the expectation for this release?

@DanGrayson DanGrayson merged commit b45f825 into Macaulay2:release-1.23-branch Mar 7, 2024
@d-torrance
Copy link
Member

Should we update the FLINT submodule to point to 3.1.0? (Not necessarily for the release, but maybe for the development branch?)

On a related note, I haven't forgotten that I volunteered to update the autotools build to use the submodule a few months ago. :)

@mikestillman
Copy link
Member

@mahrud I think maybe going forward, but not for this release (I only said you are part of the group doing releases, because you are creating the brew Macaulay2 release each time already...)

@mahrud mahrud deleted the release-1.23-branch branch March 8, 2024 18:16
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