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

update M4RI to newest upstream release #11574

Closed
malb opened this issue Jul 4, 2011 · 84 comments
Closed

update M4RI to newest upstream release #11574

malb opened this issue Jul 4, 2011 · 84 comments

Comments

@malb
Copy link
Member

malb commented Jul 4, 2011

M4RI 20110601 changed the API, so we need to apply a patch as well,

cf. https://bitbucket.org/malb/m4ri/wiki/M4RI-20110601
cf. https://bitbucket.org/malb/m4ri/wiki/M4RI-20110715

Depends on #11261
Depends on #11756

CC: @strogdon @nexttime @alexanderdreyer

Component: packages: standard

Keywords: sd32

Author: Martin Albrecht

Reviewer: Simon King, Alexander Dreyer

Merged: sage-4.7.2.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/11574

@malb malb added this to the sage-4.7.2 milestone Jul 4, 2011
@malb

This comment has been minimized.

@malb
Copy link
Member Author

malb commented Jul 4, 2011

Dependencies: #11261

@kcrisman
Copy link
Member

kcrisman commented Jul 5, 2011

comment:3

I'm a little confused. The spkgs here and at #9562 are both libm4ri, and seem to be numbered backwards (the "earlier" one in the dependency is from 2011).

@kcrisman
Copy link
Member

kcrisman commented Jul 5, 2011

comment:4

Okay, I think I figured it out. Sorry for the noise.

@kcrisman
Copy link
Member

kcrisman commented Jul 5, 2011

comment:5

I don't know what I did wrong, but I still get the problem with the Symbol not found: _m4ri_swap_bits while trying to start, even after this patch. I may have done something wrong, but at any rate I can't spend more time on this now, sorry.

@malb

This comment has been minimized.

@malb
Copy link
Member Author

malb commented Jul 6, 2011

Author: Martin Albrecht

@malb
Copy link
Member Author

malb commented Jul 7, 2011

comment:7

For the record, I tried spkg + patch on t2 and doctests pass.

@malb

This comment has been minimized.

@malb
Copy link
Member Author

malb commented Aug 15, 2011

comment:9

applies & passes doctests with 4.7.1.rc2.

@simon-king-jena
Copy link
Member

Reviewer: Simon King

@simon-king-jena
Copy link
Member

comment:10

Why is the documentation for matrices over GF(2) not part of the reference manual? I think you should use the upgrade to expose the documentation to the audience.

@malb
Copy link
Member Author

malb commented Aug 16, 2011

comment:11

I am not sure this documentation is much use in the reference manual since most of the interesting functions won't show up. Also, these matrices are expected to behave just as other matrices. On the other hand, there are a few - not many - special functions which are only available for GF(2). I'll take a look.

@malb
Copy link
Member Author

malb commented Aug 17, 2011

comment:12

It seems that you agreed in a different ticket (#9562) that this class doesn't need to be in the reference manual? Hence, returning to needs review. Feel free to change it back, though.

@simon-king-jena
Copy link
Member

comment:13

I think I can live with not including it into the references. However, currently I am getting segment faults when doctesting on mark (one of the skynet machines). I try to make sure that I properly install everything from scratch again, before reporting details.

@simon-king-jena
Copy link
Member

comment:14

I did a forced new installation of the polybori spkg, then of the M4RI spkg, and I also installed the patches. I get on mark:

sage: P.<a,b,c,d,e> = PolynomialRing(GF(2), 5, order='lex')
sage: I1 = ideal([a*b + c*d + 1, a*c*e + d*e, a*b*e + c*e, b*c + c*d*e + 1])
sage: B.<a,b,c,d,e> = BooleanPolynomialRing(5, order='lex')
sage: I2 = ideal([B(f) for f in I1.gens()])
sage: I2.groebner_basis()
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)

/home/simonking/SAGE/sage-4.7.1.rc1mark/devel/sage-main/<ipython console> in <module>()

/home/simonking/SAGE/sage-4.7.1.rc1mark/local/lib/python2.6/site-packages/sage/rings/polynomial/pbori.so in sage.rings.polynomial.pbori.BooleanPolynomialIdeal.groebner_basis (sage/rings/polynomial/pbori.cpp:26221)()

RuntimeError: Segmentation fault

Of course, it could be that the problem comes from the polybori spkg. That would be unfortunate, since it is already merged.

@simon-king-jena
Copy link
Member

comment:15

Now, I tried to force re-installation of the original polybori spkg. However, that failed as well. I hope that it will not be needed to rebuild all of sage, which takes ages on mark.

@malb
Copy link
Member Author

malb commented Aug 17, 2011

comment:16

I think the issue is that one needs to install stuff in the right order:

  • install new M4RI
  • install new PolyBoRi
  • apply the M4RI patch
  • touch sage/libs/polybori/decl.pxd
  • sage -b

@simon-king-jena
Copy link
Member

comment:17

Meanwhile I managed to revert to the original state, and can confirm that, with vanilla sage-4.7.1.rc2 on 32-bit solaris, the segfault above does not occur.

I will now try to do things in the order you suggested: In fact, originally I had installed polybori, then m4ri, then the m4ri patch, and then sage -b (without touching decl.pxd).

But how can the new polybori spkg be merged if apparently it depends on the new m4ri?

@simon-king-jena
Copy link
Member

comment:18

The new installation order did the trick, the segfault has gone. I am now repeating the failing doctests.

@simon-king-jena
Copy link
Member

comment:19

Or actually I am repeating sage -testall.

@malb
Copy link
Member Author

malb commented Aug 29, 2011

comment:67

The new M4RI SPKG is at #11757

@simon-king-jena
Copy link
Member

comment:68

Replying to @malb:

The new M4RI SPKG is at #11757

Does that mean that #11757 makes this ticket a duplicate?

@strogdon
Copy link

comment:69

Replying to @malb:

The updated patch should allow the Sage build to go through. Testing on cicero now. I also added some depends fields as suggested by Leif.

Yes, the updated patch does allow Sage to build and there are no failing tests.

@malb
Copy link
Member Author

malb commented Aug 30, 2011

comment:70

Replying to @simon-king-jena:

Replying to @malb:

The new M4RI SPKG is at #11757

Does that mean that #11757 makes this ticket a duplicate?

I guess one could look at it this way. I'd suggest: We deal with this ticket here first and if people then still have energy and are not sick of M4RI yet we can also include #11757.

@nexttime nexttime mannequin added s: needs review and removed s: needs info labels Aug 30, 2011
@malb
Copy link
Member Author

malb commented Aug 31, 2011

comment:72

So, anybody up for reviewing this? Or are people waiting for attachment: trac_11574_m4ri_sse2.patch being ported to #11757?

@strogdon
Copy link

strogdon commented Sep 1, 2011

comment:73

Replying to @malb:

So, anybody up for reviewing this? Or are people waiting for attachment: trac_11574_m4ri_sse2.patch being ported to #11757?

I should probably defer review to Simon since I got in through the backdoor. However,
since Sage builds here and there are no doctest failures I would give it a positive review. I do have one question. Shouldn't this ticket depend upon ticket #11756, even though that one has a positive review?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 1, 2011

comment:74

Replying to @strogdon:

Shouldn't this ticket depend upon ticket #11756, even though that one has a positive review?

Yep, IMHO even if #11756 had already been merged into some devel release.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 1, 2011

Changed dependencies from #11261 to #11261 #11756

@simon-king-jena
Copy link
Member

comment:75

Replying to @strogdon:

Replying to @malb:

So, anybody up for reviewing this? Or are people waiting for attachment: trac_11574_m4ri_sse2.patch being ported to #11757?

I should probably defer review to Simon since I got in through the backdoor.

Sorry, I will probably not be able to review it in the next couple of days. So, iI'd appreciate if someone was quicker than I.

@malb
Copy link
Member Author

malb commented Sep 6, 2011

comment:76

anyone?

@malb

This comment has been minimized.

@alexanderdreyer
Copy link
Mannequin

alexanderdreyer mannequin commented Sep 7, 2011

comment:77

The spkg builds and and installs fine. Together with the patches sage -testall was run successfully on a SuSE Linux Enterprise 11 x86_64, Gentoo i486, and Mac OS X 10.5 ppc.

So, I can cordially give a positive review!

@alexanderdreyer
Copy link
Mannequin

alexanderdreyer mannequin commented Sep 7, 2011

Changed reviewer from Simon King to Simon King, Alexander Dreyer

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2011

comment:78

attachment: m4ri_20110601.patch has no ticket number in its commit message.

@malb
Copy link
Member Author

malb commented Sep 12, 2011

rebased to 4.7.1.alpha4

@malb
Copy link
Member Author

malb commented Sep 12, 2011

comment:79

Attachment: m4ri_20110601.patch.gz

fixed.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2011

Merged: sage-4.7.2.alpha3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants