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

porting to python 3? #1

Open
dimpase opened this issue May 6, 2020 · 10 comments
Open

porting to python 3? #1

dimpase opened this issue May 6, 2020 · 10 comments

Comments

@dimpase
Copy link

dimpase commented May 6, 2020

I'm getting

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.1.rc2, Release Date: 2020-04-25                 │
│ Using Python 3.7.3. Type "help()" for help.                        │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: import drg
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-608957877e7c> in <module>()
----> 1 import drg

/home/dimpase/work/sage-drg/drg/drg.py in <module>()
     11 from sage.symbolic.relation import solve as _solve
     12 from sage.symbolic.ring import SR
---> 13 from .array3d import Array3D
     14 from .assoc_scheme import ASParameters
     15 from .assoc_scheme import PolyASParameters

ImportError: attempted relative import with no known parent package

As you see, I am launching Sage in the package directory, so this ought to work (?) - but it does not. As we basically switched to Python 3 (the forthcoming release 9.1 will be the last to support both python 2 and 3), this seems to need some work.

But it could also be that I'm doing something silly and miss a necessary step before this is meant to work.

@dimpase
Copy link
Author

dimpase commented May 6, 2020

@Ivo-Maffei - does it work for you?

@Ivo-Maffei
Copy link

Ivo-Maffei commented May 6, 2020

I got it working by doing the following:

  1. copy drg folder in sage/src/sage/
  2. build sage (sage -b)
  3. import sage.drg as drg

Then I tried the examples from the README.md and they seem to work.

@dimpase
Copy link
Author

dimpase commented May 6, 2020

Doesn't work for me (I gather some imports are needed to be added)
@Ivo-Maffei - could you perhaps provide a git branch (against the latest Sage 9.1.rc3, preferabley) with this working (in Python 3)?

@jaanos
Copy link
Owner

jaanos commented May 6, 2020

I should be providing an option to make the package available for pip installation. I have this planned - but first, I intend to write a proper documentation.

The easiest way you can get the package to work is to run Sage from the repository root (not from the drg subfolder).

Alternatively, if you want the package to be available regardless of how Sage is run, you can copy (or symlink) the drg folder into sage/local/lib/python3.7/site-packages/ (which is what a sage -pip installation would do). No Sage building is then needed.

Either way, you can then do things like

import drg
from drg import DRGParameters

This will import the entire module, not just drg.py, as the error suggested.

Although the package has been developed with Python 2, I have done some Python 3 porting, so at the very least, the examples in this repository should work (note that the Binder link in README.md runs on Sage 9.0). But it is possible that I may have missed something - in that case, I would appreciate any error reports.

@dimpase
Copy link
Author

dimpase commented May 6, 2020

Right, a symlink to site-packages works, thanks.

Have you looked into making this package a part of sagelib?

@jaanos
Copy link
Owner

jaanos commented May 10, 2020

Have you looked into making this package a part of sagelib?

If I understand correctly, this would make the package part of Sage itself?

I have thought of contributing the integral solution finder to Sage, as it seems that it could be more widely useful. Of course, doing so would mean making the interface more friendly.

@dimpase
Copy link
Author

dimpase commented May 14, 2020

If I understand correctly, this would make the package part of Sage itself?

yes - it's certainly possible and welcome.

I have thought of contributing the integral solution finder to Sage, as it seems that it could be more widely useful. Of course, doing so would mean making the interface more friendly.

Isn't this merely enumerates all the integer points in a polytope? Sage has a number of ways for doing it already, e.g. http://doc.sagemath.org/html/en/reference/discrete_geometry/sage/geometry/polyhedron/ppl_lattice_polytope.html

The question is whether your code for this task has any advantage over these.

@jaanos
Copy link
Owner

jaanos commented May 14, 2020

Isn't this merely enumerates all the integer points in a polytope? Sage has a number of ways for doing it already, e.g. http://doc.sagemath.org/html/en/reference/discrete_geometry/sage/geometry/polyhedron/ppl_lattice_polytope.html

Yes, this is it essentially - yet when I tried using integral point enumeration, it proved to be unsuitable for my purposes. My code provides a generator/coroutine instead of outputting all the solutions, and it also supports adding conditions on-the-fly (as well as producing a copy of the generator in the current state). I have used mixed integer linear programming, but if polytope enumeration can be made faster, I guess it could also be used (although it seems that the current algorithm simply iterates through all integral points in a bounding box, so I'm not sure that this is going to be faster than MILP).

Anyway, I believe that Sage does need a simple method to do this (similar to Mathematica's FindInstance - although the latter is more general) instead of forcing the user to restate as a different problem, whatever the method used behind the scenes.

@dimpase
Copy link
Author

dimpase commented May 14, 2020 via email

@jaanos
Copy link
Owner

jaanos commented May 14, 2020

Of course - as I have said, the interface would also need to be rewritten so that talking back to the generator is done with appropriate methods and not just send.

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

No branches or pull requests

3 participants