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 4ti2 to version 1.6 #15172

Closed
dimpase opened this issue Sep 7, 2013 · 17 comments
Closed

update 4ti2 to version 1.6 #15172

dimpase opened this issue Sep 7, 2013 · 17 comments

Comments

@dimpase
Copy link
Member

dimpase commented Sep 7, 2013

version 1.6 is released.

Intsall the updated spkg and apply attachment: 42.patch

CC: @novoselt @miguelmarco @sagetrac-dperkinson @sagetrac-mhampton

Component: packages: optional

Keywords: 4ti2

Author: Dmitrii Pasechnik

Reviewer: Volker Braun

Merged: sage-5.13.beta1

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

@dimpase dimpase added this to the sage-5.13 milestone Sep 7, 2013
@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

comment:1

at the moment there is one test failure, which only occurs from sage -t run, but I can't reproduce it interactively:

$ ../../sage -t --optional='sage,4ti2' sage/interfaces/four_ti_2.py
Running doctests with ID 2013-09-07-09-09-54-b455bb50.
Doctesting 1 file.
sage -t sage/interfaces/four_ti_2.py
**********************************************************************
File "sage/interfaces/four_ti_2.py", line 284, in sage.interfaces.four_ti_2.FourTi2.call
Failed example:
    four_ti_2.read_matrix("test_file.gro") # optional - 4ti2
Expected:
    [-5  0  2]
    [-5  3  0]
Got:
    []
**********************************************************************
1 item had failures:
   1 of   5 in sage.interfaces.four_ti_2.FourTi2.call
    [50 tests, 1 failure, 0.55 s]
----------------------------------------------------------------------
sage -t sage/interfaces/four_ti_2.py  # 1 doctest failed

@dimpase

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Sep 7, 2013

comment:4

Why add # optional - 4ti2 where it was not needed before?

@jdemeyer
Copy link

jdemeyer commented Sep 7, 2013

comment:5

The following

        cwd = os.getcwd() # Save working directory in order to restore it.
        os.chdir(self.directory())

        subprocess.call(cmd, shell=True)

        os.chdir(cwd) # Restore previous working directory.

should be replaced by

        subprocess.call(cmd, shell=True, cwd=self.directory())

@novoselt
Copy link
Member

novoselt commented Sep 7, 2013

comment:6

As I understand in the patch "-q" is for quiet mode, but why there is a need to silence stderr explicitly? Shouldn't commands output no errors if used correctly?

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

comment:7

Replying to @novoselt:

As I understand in the patch "-q" is for quiet mode, but why there is a need to silence stderr explicitly? Shouldn't commands output no errors if used correctly?

there is no '-q' switch for ppi, for some reason. So its stderr output needs to be silenced.

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

comment:8

Replying to @jdemeyer:

Do you mean it's an equivalent way to do the same thing?

OK, I'll change this. Meanwhile I just uploaded patch which fixes the issue that was stopping me here; it's actually due to filename semantics changed. Now groebner bla.foo appends the suffix gro to bla.foo, rather than replacing the suffix.

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

Changed work issues from tmp_dir related doctest issue to none

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

Attachment: 42.patch.gz

updated patch

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

comment:9

OK, it looks as if I fixed everything.

@dimpase
Copy link
Member Author

dimpase commented Sep 7, 2013

comment:10

There are quite a few changes in the 4ti2-dependent part of sandpile.py, due to a change in output format of zslove, which prints matrix dimensions on one row now, and changed order in its output. Also, I needed to take care of deprecated vertex_set in SimplicialComplex.

@jdemeyer
Copy link

jdemeyer commented Sep 7, 2013

comment:11

Replying to @dimpase:

Replying to @jdemeyer:

Do you mean it's an equivalent way to do the same thing?

Essentially yes. Instead of changing directory globally and then changing it back, we simply change directory for the executed command.

@dimpase
Copy link
Member Author

dimpase commented Sep 9, 2013

comment:12

I've made a small change in spkg-install. Now everything, not only executables, is installed, so that one can link against 4ti2's libraries.

@vbraun
Copy link
Member

vbraun commented Oct 6, 2013

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Oct 6, 2013

comment:14

Looks good to me.

@vbraun
Copy link
Member

vbraun commented Oct 6, 2013

Author: Dmitrii Pasechnik

@jdemeyer
Copy link

Merged: sage-5.13.beta1

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

4 participants