Opened 6 years ago

Closed 6 years ago

#15172 closed enhancement (fixed)

update 4ti2 to version 1.6

Reported by: dimpase Owned by:
Priority: major Milestone: sage-5.13
Component: packages: optional Keywords: 4ti2
Cc: novoselt, mmarco, dperkinson, mhampton Merged in: sage-5.13.beta1
Authors: Dmitrii Pasechnik Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by dimpase)

version 1.6 is released.

Intsall the updated spkg and apply 42.patch

Attachments (1)

42.patch (7.5 KB) - added by dimpase 6 years ago.
updated patch

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by dimpase

  • Description modified (diff)
  • Type changed from defect to enhancement

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

comment:2 Changed 6 years ago by dimpase

  • Cc novoselt mmarco added
  • Status changed from new to needs_info

comment:3 Changed 6 years ago by dimpase

  • Status changed from needs_info to needs_work

comment:4 Changed 6 years ago by jdemeyer

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

comment:5 follow-up: Changed 6 years ago by jdemeyer

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())

comment:6 follow-up: Changed 6 years ago by 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?

comment:7 in reply to: ↑ 6 Changed 6 years ago by dimpase

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.

comment:8 in reply to: ↑ 5 ; follow-up: Changed 6 years ago by dimpase

  • Work issues tmp_dir related doctest issue deleted

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.

Changed 6 years ago by dimpase

updated patch

comment:9 Changed 6 years ago by dimpase

  • Status changed from needs_work to needs_review

OK, it looks as if I fixed everything.

comment:10 Changed 6 years ago by dimpase

  • Cc dperkinson added

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.

comment:11 in reply to: ↑ 8 Changed 6 years ago by jdemeyer

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.

comment:12 Changed 6 years ago by dimpase

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.

comment:13 Changed 6 years ago by dimpase

  • Cc mhampton added

comment:14 Changed 6 years ago by vbraun

  • Authors set to Dmitrii Pasechnik
  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me.

comment:15 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.13.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.