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

Upgrade to Tachyon 0.99.5 #23712

Closed
dimpase opened this issue Aug 25, 2017 · 74 comments · Fixed by #36969
Closed

Upgrade to Tachyon 0.99.5 #23712

dimpase opened this issue Aug 25, 2017 · 74 comments · Fixed by #36969

Comments

@dimpase
Copy link
Member

dimpase commented Aug 25, 2017

We currently are shipping a 12 years old tachyon (0.98.9).

0.99.2 was released Dec 2021. http://jedi.ks.uiuc.edu/~johns/raytracer/files/

This is also needed to make Sage work on arm64/aarch64, see #23687.

Many distributions are still on 0.99.beta6 - https://repology.org/project/tachyon/versions

In Debian:

CC: @slel @orlitzky @kiwifb @antonio-rojas @tscrim

Component: packages: standard

Keywords: upgrade, tachyon

Author: Dima Pasechnik, Gonzalo Tornaría

Branch/Commit: u/dimpase/packages/tachyon0992 @ d3034fc

Reviewer: Matthias Koeppe

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

@dimpase dimpase added this to the sage-8.1 milestone Aug 25, 2017
@fchapoton
Copy link
Contributor

comment:1

None of our 4 patches does apply. I do not know if they should be removed or rebased.


New commits:

8138ccfnew tachyon 99b6

@fchapoton
Copy link
Contributor

Commit: 8138ccf

@fchapoton
Copy link
Contributor

Branch: public/23712

@dimpase
Copy link
Member Author

dimpase commented Aug 26, 2017

comment:2

there are quite a few doctests failing with this branch. Presumably the interface might have changed a bit.

@kiwifb
Copy link
Member

kiwifb commented Aug 27, 2017

comment:3

Replying to @dimpase:

there are quite a few doctests failing with this branch. Presumably the interface might have changed a bit.

That's what I remember. Are all the failing doctests in doc? I realised yesterday that I have lost the ability to test those in sage-on-gentoo recently (for some reason .rst files get installed as .rst.txt and then they are not tested). So I miss all broken doctests from rst files at the moment.

For the record I don't seem to have any (repeatable) failures on the stuff I doctest in sage-on-gentoo.

@dimpase
Copy link
Member Author

dimpase commented Aug 27, 2017

comment:4

This is what I get with ptestlong:

sage -t --long --warn-long 45.7 src/sage/plot/plot3d/parametric_plot3d.py  # 1 doctest failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/implicit_plot3d.py  # 6 doctests failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/platonic.py  # 1 doctest failed
sage -t --long --warn-long 45.7 src/sage/graphs/generic_graph.py  # 9 doctests failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/parametric_surface.pyx  # 3 doctests failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/implicit_surface.pyx  # 1 doctest failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/tachyon.py  # 21 doctests failed
sage -t --long --warn-long 45.7 src/sage/combinat/tiling.py  # 1 doctest failed
sage -t --long --warn-long 45.7 src/sage/numerical/sdp.pyx  # 9 doctests failed
sage -t --long --warn-long 45.7 src/sage/numerical/backends/cvxopt_sdp_backend.pyx  # 7 doctests failed
sage -t --long --warn-long 45.7 src/sage/geometry/polyhedron/backend_normaliz.py  # 2 doctests failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/index_face_set.pyx  # 1 doctest failed
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/base.pyx  # 1 doctest failed

Typically it's something about PNG interface:

Using --optional=ccache,cmake,database_gap,gap_packages,mpir,normaliz,pynormaliz,python2,qhull,sage
Doctesting entire Sage library.
Sorting sources by runtime so that slower doctests are run first....
Doctesting 3578 files using 4 threads.
sage -t --long --warn-long 45.7 src/sage/plot/plot3d/parametric_plot3d.py
**********************************************************************
File "src/sage/plot/plot3d/parametric_plot3d.py", line 172, in sage.plot.plot3d.parametric_plot3d.?
Failed example:
    P.show(viewer='tachyon')
Exception raised:
    Traceback (most recent call last):
      File "/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.plot.plot3d.parametric_plot3d.?[13]>", line 1, in <module>
        P.show(viewer='tachyon')
      File "sage/plot/plot3d/base.pyx", line 1478, in sage.plot.plot3d.base.Graphics3d.show (build/cythonized/sage/plot/plot3d/base.c
:21367)
        dm.display_immediately(self, **kwds)
      File "/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/sage/repl/rich_output/display_manager.py", line 834, in displa
y_immediately
        self._backend.display_immediately(plain_text, rich_output)
      File "/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/sage/repl/rich_output/backend_doctest.py", line 209, in displa
y_immediately
        self.validate(rich_output)
      File "/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/sage/repl/rich_output/backend_doctest.py", line 270, in valida
te
        assert rich_output.png.get().startswith('\x89PNG')
    AssertionError

Running the corresponding lines at the Sage prompt works and shows a meaningful plot.
However, it's not stored in a PNG file, it's stored in TGA file:

$ file tmp_ZccmzF.png 
tmp_ZccmzF.png: Targa image data - RGB 500 x 500 x 24 - top Targa image data - RGB 500 x 500 x 24 - top

@dimpase
Copy link
Member Author

dimpase commented Aug 27, 2017

comment:5

In fact, ldd says that tachyon is not linked to libpng - no wonder it cannot do PNG.

@dimpase
Copy link
Member Author

dimpase commented Aug 27, 2017

comment:6

I see that Debian has a set of patches to this version, including autotoolisation!
https://packages.debian.org/source/sid/tachyon

Should we use them, rather than keep wrestling with patches to Makefiles?

@kiwifb
Copy link
Member

kiwifb commented Aug 27, 2017

comment:7

Probably. The linking problem would also explain why I don't see those failures in s-o-g.

@kiwifb
Copy link
Member

kiwifb commented Aug 28, 2017

comment:8

That's a lot of patches https://anonscm.debian.org/cgit/debian-science/packages/tachyon.git/tree/debian/patches but most of them will probably be handy. Are you up to write a spkg-src?

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2017

comment:9

Replying to @kiwifb:

That's a lot of patches https://anonscm.debian.org/cgit/debian-science/packages/tachyon.git/tree/debian/patches but most of them will probably be handy. Are you up to write a spkg-src?

I have a problem with them at the moment: namely, applying these patches and invoking autoreconf -i -f leads to

src/Makefile.am:33: error: HAVE_LD_VERSION_SCRIPT does not appear in AM_CONDITIONAL

Note that src/Makefile.am has the following lines around line 33:

libtachyon_la_LD_VERSION_SCRIPT=
if HAVE_LD_VERSION_SCRIPT
libtachyon_la_LD_VERSION_SCRIPT+= -Wl,--version-script=$(top_srcdir)/src/tachyon.map
endif

This can be remedied by adding to configure.ac the following:

AM_CONDITIONAL([HAVE_LD_VERSION_SCRIPT],   [blah...])

but I don't quite see what blah... should be. (I put in some random shell call just to see if it helps, and it does, but that's of course not a proper fix).

So this looks like a bug in Debian patches (namely in upstream-rationalization-autotools.patch), which only manifests itself in non-Debian setting.

@dimpase
Copy link
Member Author

dimpase commented Aug 28, 2017

comment:10

And I also don't get how to configure and run make to create a tachyon executable. All what make install does, it installs headers and various dynamic libs, such as libtachyon-mt-thr.so.0.0.0.

@slel
Copy link
Member

slel commented Jan 30, 2019

Changed keywords from none to upgrade, tachyon

@slel

This comment has been minimized.

@slel slel modified the milestones: sage-8.1, sage-8.7 Jan 30, 2019
@slel slel changed the title update tachyon to 0.99 Upgrade to Tachyon 0.99b6 Jan 30, 2019
@embray
Copy link
Contributor

embray commented Mar 25, 2019

comment:12

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

@embray embray modified the milestones: sage-8.7, sage-8.8 Mar 25, 2019
@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:13

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

@embray embray removed this from the sage-8.8 milestone Jun 14, 2019
@timokau
Copy link
Contributor

timokau commented Aug 13, 2019

comment:14

For what it's worth I'm just fixing up aarch64 sage on nixos and I get no doctest failures with tachyon 0.99b2 (the version we ship).

There are some transient timeout failures, but that's it. Probably not related to tachyon.

@orlitzky
Copy link
Contributor

comment:15

There's officially a new release, v0.99.2:

http://jedi.ks.uiuc.edu/~johns/raytracer/

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Upgrade to Tachyon 0.99b6 Upgrade to Tachyon 0.99.2 Mar 6, 2022
@dimpase
Copy link
Member Author

dimpase commented Mar 6, 2023

I can't rename anything there, it's @vbraun who can

@tornaria
Copy link
Contributor

@vbraun or @haraldschilly : could you please fix the upstream source file tachyon-0.99.5.tar.gz?

This file is available in sagemath mirror, e.g. https://mirrors.mit.edu/sage/spkg/upstream/tachyon/index.html, but it is incorrectly named tachyon-0.99.5.tar.bz2.

This is the original file with the original timestamp that I downloaded from https://web.archive.org/web/20220523051055/http://jedi.ks.uiuc.edu/~johns/tachyon/files/0.99.5/:

$ TZ=UTC ls --full-time tachyon-0.99.5.tar.gz 
-rw-r--r-- 1 tornaria tornaria 1163063 2022-04-23 15:06:53.000000000 +0000 tachyon-0.99.5.tar.gz
$ sha256sum tachyon-0.99.5.tar.gz 
09203c102311149f5df5cc367409f96c725742666d19c24db5ba994d5a81a6f5  tachyon-0.99.5.tar.gz
$ nix --extra-experimental-features nix-command hash file tachyon-0.99.5.tar.gz 
sha256-CSA8ECMRFJ9d9cw2dAn5bHJXQmZtGcJNtbqZTVqBpvU=
$ md5sum tachyon-0.99.5.tar.gz 
378156b54842fc91791daea6dd6f1a72  tachyon-0.99.5.tar.gz

As a reference, the sha256sum matches the one in archlinux and in nixpkgs.

The file that it is now available in the sagemath upstream mirror is the correct one, but the filename is not the original one (neither is the timestamp).

@mkoeppe mkoeppe modified the milestones: sage-10.0, sage-10.1 Apr 30, 2023
@mkoeppe mkoeppe removed this from the sage-10.1 milestone Aug 7, 2023
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 13, 2023

the support for newer tachyon is already merged so it should be possible now to upgrade without trouble

Let's restart this effort for Sage 10.3.

Current version in Sage gives an error with conda-forge-python3.11-minimal https://github.com/sagemath/sage/actions/runs/6842945144/job/18604992673#step:11:2493

  [tachyon-0.98.9.p7]   /opt/conda/bin/x86_64-conda-linux-gnu-cc -m64 -Wall -O3 -fomit-frame-pointer -ffast-math -DLinux -DLP64 -DTHR -D_REENTRANT  -DUSEPNG    -I/sage/local/include  -c ../src/pngfile.c -o ../compile/linux-64-thr/libtachyon/pngfile.o
  [tachyon-0.98.9.p7]   ../src/pngfile.c:33:10: fatal error: png.h: No such file or directory
  [tachyon-0.98.9.p7]      33 | #include "png.h" /* the libpng library headers */
  [tachyon-0.98.9.p7]         |          ^~~~~~~
  [tachyon-0.98.9.p7]   compilation terminated.
  [tachyon-0.98.9.p7]   make[5]: *** [Makefile:317: ../compile/linux-64-thr/libtachyon/pngfile.o] Error 1

... and I'd rather debug it for the new version than for the ancient version...

@mkoeppe mkoeppe added this to the sage-10.3 milestone Nov 13, 2023
mkoeppe pushed a commit to mkoeppe/sage that referenced this issue Dec 26, 2023
@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 26, 2023

I can confirm by md5 that it is the same file I have from upstream. However, the original filename is tachyon-0.99.5.tar.gz and it is indeed compressed with gzip.

@tornaria In #36959, I have changed the tarball attribute to the correct file extension, so a new file tachyon-0.99.5.tar.gz will be uploaded to the mirrors when this PR is merged.

@mkoeppe mkoeppe removed this from the sage-10.3 milestone Mar 5, 2024
@aikrahguzar
Copy link
Contributor

I am running into problems with building sage on aarch64 linux due to tachyon. The buid failed with,

[spkg-install] Error: Sorry, your platform isn't supported by Tachyon and/or Sage. Exiting...

when trying to install tachyon-0.98.9.p7

@dimpase
Copy link
Member Author

dimpase commented Jul 25, 2024

@aikrahguzar - can you try the branch here?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 25, 2024

Or rather #36969

mkoeppe pushed a commit to mkoeppe/sage that referenced this issue Jul 25, 2024
@dimpase
Copy link
Member Author

dimpase commented Jul 25, 2024

@aikrahguzar - does your OS have a tachyon package ? If so, can you install it, and tell Sage to use it (running ./configure again).

Or else you can install tachyon into /usr/local, and
tell Sage to use it (running ./configure again).

@aikrahguzar
Copy link
Contributor

Or rather #36969

I tried, this time build progresses further but still fails with,

[spkg-install] make[5]: *** No rule to make target 'linux-arm-thr'.  Stop.

I am attaching the full log too:
tachyon-0.99.5.log.txt

@aikrahguzar
Copy link
Contributor

@aikrahguzar - does your OS have a tachyon package ? If so, can you install it, and tell Sage to use it (running ./configure again).

Fedora doesn't seem to have a tachyon package. At least dnf search tachyon doesn't return anything.

Or else you can install tachyon into /usr/local, and tell Sage to use it (running ./configure again).

I will try the manual source installation of tachyon later.

@dimpase
Copy link
Member Author

dimpase commented Jul 25, 2024

this looks like tachyon needs a patch to build on such a system.

Hopefully just a Makefile target, which would be the same as for an aarch64 linux, only named differently.

@aikrahguzar
Copy link
Contributor

aikrahguzar commented Jul 25, 2024

this looks like tachyon needs a patch to build on such a system.

Hopefully just a Makefile target, which would be the same as for an aarch64 linux, only named differently.

Almost only result I could find for linux-arm-thr was a sage patch for tachyon. The patch doesn't apply but after clumsily messing about with the following allows tachyon to build:
Make-arch.patch

The build is now continuing.

Edit: I am on aarch64 linux. I think linux-arm-thr is what tachyon calls it.

Edit: I have a working build of sage now. Thanks a lot @mkoeppe and @dimpase

@dimpase
Copy link
Member Author

dimpase commented Jul 25, 2024

@aikrahguzar - were you still using tachyon 0.98, but added this patch?

or you needed it with the branch updating to 0.99.5 ?

@aikrahguzar
Copy link
Contributor

@aikrahguzar - were you still using tachyon 0.98, but added this patch?

or you needed it with the branch updating to 0.99.5 ?

The later i.e. I added the patch on top of #36969

@dimpase
Copy link
Member Author

dimpase commented Jul 25, 2024

@aikrahguzar - were you still using tachyon 0.98, but added this patch?

or you needed it with the branch updating to 0.99.5 ?

The later i.e. I added the patch on top of #36969

please add a review to #36969 mentioning this

vbraun pushed a commit to vbraun/sage that referenced this issue Jul 31, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

Continuation of sagemath#23712

Author: @dimpase, @tornaria, @aikrahguzar

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
Resolves sagemath#23712
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36969
Reported by: Matthias Köppe
Reviewer(s): aikrahguzar, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Aug 2, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

Continuation of sagemath#23712

Author: @dimpase, @tornaria, @aikrahguzar

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
Resolves sagemath#23712
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36969
Reported by: Matthias Köppe
Reviewer(s): aikrahguzar, Matthias Köppe
@vbraun vbraun closed this as completed in 9d71411 Aug 3, 2024
@mkoeppe mkoeppe added this to the sage-10.5 milestone Aug 3, 2024
MingcongBai pushed a commit to AOSC-Tracking/sage that referenced this issue Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.