Closed mmasdeu closed 8 years ago
Branch: u/mmasdeu/20388
I was able to use it! Note that on the server I have access to I need two commands to launch magma
$ module load magma/2.11.13 # Slurm
$ magma
And Magma(server='plafrim', command='module load magma/2.11.13; magma')
worked perfectly. Do you think it is worth it to add in the doc?
Sided note: Would be nice for users to set up their configurations once for all for these external interfaces (via environment variables?). For example, I have access to maple and magma but they live on two different servers.
That's great! I added the 'command' parameter because it could be potentially useful although I didn't need it. I am happy to see that my guess was correct.
About setting defaults, have you tried to put
magma = Magma(server='plafrim', command='module load magma/2.11.13; magma')
maple = (whatever is pertinent, I don't have access to maple)
in $HOME/.sage/init.sage?
Finally, although I agree that the documentation should advertise more this feature, I don't know where to put it exactly.
Setting them in init.sage
is indeed a solution to make magma(something)
works out of the box.
My question about default configuration did not make much sense since I thought that SageObject
would have methods _magma_
and _maple_
. If it was
def _magma_(self, G=None):
if G is None:
import sage.interfaces.magma
G = sage.interfaces.magma.magma
return self._interface_(G)
then it would not take into account any specific server configuration.
Branch pushed to git repo; I updated commit sha1. New commits:
da0c8a1 | Fixed two functions that were using the default magma. |
Thanks for testing it! I found two functions (one in arith.all, one in plane_conics/con_field.py) that imported magma. I have changed them to try to use the global magma if it exists (which should be the case if the user initialized it, anyway).
A few things you might want to look at:
ext/magma
requires file access (it's written as a module). However, one could put those files within reach of magma via other means. The "scp" variant possibly works quite often, so it may be a reasonable default thing to try, but we should probably not fail if it doesn't work.Relying on globals
is not very safe
sage: magma = MyMagmaStructure()
sage: bernoulli(3, algorithm='magma') # boom
I think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like
sage: sage.interfaces.config('magma', server=X', command='Y')
Replying to @videlec:
I think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like
sage: sage.interfaces.config('magma', server=X', command='Y')
I would hang those properties on sage.interfaces.magma.magma
, which is the "default" instantiation of the magma interface.
Note that sage.interfaces.magma.magma
is instantiated upon load, so we're too late to set defaults for its instantiation. However, actually starting up only happens once the interface really gets used, so you could just adapt the relevant properties on the magma object before using the interface.
A workaround you can already do if you want to use a magma with non-standard setting is "monkey patch" the global instance, i.e.,
sage: sage.interfaces.magma.magma = Magma(...<options you want>...)
with the problem that if some other file (such as sage.interfaces.all
) has done an import from
, you're not reaching them. So perhaps it's a little more robust to adjust attributes on sage.interfaces.magma.magma
rather than rebind it to a different instance (sigh ... and then they say you don't have to think about pointers when working in python)
I have addressed Nils' two concerns above. About the "magma = Magma()" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:
sage: set_magma_session(Magma(server = 'remote', command = 'magma_custom'))
sage: function_using_magma(algorithm = 'magma')
This would only involve changing the calls that look like:
from sage.interfaces.magma import magma
magma.eval('complicated stuff')
to
from sage.interfaces.magma import get_magma_session
magma = get_magma_session()
magma.eval('complicated stuff')
Do you think that this is a good enough solution? If so, I would quickly implement it...
Replying to @mmasdeu:
I have addressed Nils' two concerns above. About the "magma = Magma()" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:
sage: set_magma_session(Magma(server = 'remote', command = 'magma_custom')) sage: function_using_magma(algorithm = 'magma')
This would only involve changing the calls that look like:
from sage.interfaces.magma import magma magma.eval('complicated stuff')
to
from sage.interfaces.magma import get_magma_session magma = get_magma_session() magma.eval('complicated stuff')
Do you think that this is a good enough solution? If so, I would quickly implement it...
I do not like it so much. It would involve a lot of changes in the schemes/ folder (that people have maybe already copy/paste in their personal code). And, more importantly, it would be different from any other interface in Sage. What about environment variable?
$ SAGE_MAGMA_COMMAND = "module load magma/2.11.13; magma"
$ SAGE_MAGMA_SERVER = "plafrim"
$ export SAGE_MAGMA_COMMAND SAGE_MAGMA_SERVER
$ sage
...
One just needs in the constructor of Magma
something like
class Magma(Interface):
def __init__(self, server=None, command=None, ...):
import os
if server is None:
server = os.getenv('SAGE_MAGMA_SERVER')
if command is None:
command = os.getenv('SAGE_MAGMA_COMMAND')
A solution based on init.sage
would not work since it is loaded after anything else.
1) Some users may not be familiar with environment variables. 2) I like to be able to write Sage scripts that work after you start sage. Using environment variables forces you to remember always to set them before you start your script, which can be annoying. 3) The environment variable approach doesn't allow the user to compare the result of executing a function with two different Magma installations. Something like this would be desiderable.
sage: E = EllipticCurve('37a1')
sage: set_magma_session(Magma(command = 'magma_2-18'))
sage: E.analytic_rank(algorithm = 'magma')
0
sage: set_magma_session(Magma(command = 'magma_2-19'))
sage: E.analytic_rank(algorithm = 'magma')
0
4) I don't understand your first comment. If people has taken Sage code that uses Magma and now Sage adds more functionality, then they can either use the old functionality or adapt the code to the new (and better way), right?
Here is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?
Replying to @mmasdeu:
Here is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?
You are right that it is desirable to have a way to modify it from Sage itself. However, I would not populate the global namespace with get_magma_session
unless the global magma
is deprecated (which can be done using lazy imports).
Moreover, it is very confusing to have all of: magma
, Magma
, magma_free
, magma_console
, magma_version
in the global namespace... (not talking about Magmas
). A cleanup is desirable.
I think the set_magma_session
is a rather heavy interface. It seems to me the appropriate place would be a method on sage.interfaces.expect.Expect
along the lines of
def set_server_and_command(self,server,command):
if self._expect:
raise RuntimeError("interface has already started")
self._server = server
self.__command = command
If there are other settings to change as well, it may be necessary to override this routine on Magma (and do the usual super() call thing)
The main thing is that an instantiated expect interface doesn't run a process on its own. Starting it is a separate operation (and in fact, during the lifetime of an expect object it may start and stop underlying processes repeatedly)
The cleaner solution would be to make new Magma
instances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session.
Replying to @nbruin:
I think the
set_magma_session
is a rather heavy interface. It seems to me the appropriate place would be a method onsage.interfaces.expect.Expect
along the lines ofdef set_server_and_command(self,server,command): if self._expect: raise RuntimeError("interface has already started") self._server = server self.__command = command
I like better this approach. That way the server can even be configured in init.sage
!
The cleaner solution would be to make new
Magma
instances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session.
I would actually remove the Magma
from the global namespace. That would not prevent anyone from testing different versions.
Changed branch from u/mmasdeu/20388 to u/mmasdeu/20388-fix
Tested succesfully with magma
and matlab
! Great.
Though, there is something wrong with maple
(not sure where and how it might be related to the changes in this ticket)
sage: maple.set_server_and_command('calcul1')
sage: maple('2+2') # hang out infinitely
And even after a Ctrl-C I do not have back the sage prompt (only ^CInterrupting Maple...
). After a second Ctrl-C I get the prompt back with the following traceback
...
.../interfaces/maple.pyc in set(self, var, value)
619 """
620 cmd = '%s:=%s:' % (var, value)
--> 621 out = self.eval(cmd)
622 if out.find("error") != -1:
623 raise TypeError("Error executing code in Maple\nCODE:\n\t%s\nMaple ERROR:\n\t%s" % (cmd, out))
.../interfaces/expect.pyc in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds)
1258 elif split_lines:
1259 return '\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds)
-> 1260 for L in code.split('\n') if L != ''])
1261 else:
1262 return self._eval_line(code, allow_use_file=allow_use_file, **kwds)
.../interfaces/maple.pyc in _eval_line(self, line, allow_use_file, wait_for_prompt, restart_if_needed)
571 with gc_disabled():
572 z = Expect._eval_line(self, line, allow_use_file=allow_use_file,
--> 573 wait_for_prompt=wait_for_prompt).replace('\\\n','').strip()
574 if z.lower().find("error") != -1:
575 raise RuntimeError("An error occurred running a Maple command:\nINPUT:\n%s\nOUTPUT:\n%s" % (line, z))
...
(and maple
works perfectly well on calcul1
through ssh
).
Might be too much, but we could even move the environment part inside pexpect
server = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))
What do you think?
To fit with coding conventions could change
(self,server = None,command = None, server_tmpdir = None, ulimit = None)
-->
(self, server=None, command=None, server_tmpdir=None, ulimit=None)
And add cross doctests between set_server_and_command
, server
and command
.
An annoying feature
sage: magma.set_<TAB>
Creating list of all Magma intrinsics for use in tab completion.
...
Scanning Magma types ...
Branch pushed to git repo; I updated commit sha1. New commits:
8a28c1a | Added examples. Moved environment variables to expect. |
Replying to @videlec:
An annoying feature
sage: magma.set_<TAB> Creating list of all Magma intrinsics for use in tab completion. ... Scanning Magma types ...
This only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...
Replying to @videlec:
Though, there is something wrong with
maple
(not sure where and how it might be related to the changes in this ticket)
I have no easy way to debug this. Maybe open a new ticket?
Done!
Replying to @videlec:
Might be too much, but we could even move the environment part inside pexpect
server = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))
What do you think?
To fit with coding conventions could change
(self,server = None,command = None, server_tmpdir = None, ulimit = None) --> (self, server=None, command=None, server_tmpdir=None, ulimit=None)
And add cross doctests between
set_server_and_command
,server
andcommand
.
We have a strange message before the Sage prompt if we do not specify the tmp repository
$ export SAGE_MAPLE_SERVER='calcul1'
$ sage
SageMath version ...
No remote temporary directory (option server_tmpdir) specified,
using /tmp/ on calcul1
sage:
Do you think this is ok? Everything is fine with
$ export SAGE_MAPLE_SERVER='calcul1'
$ export SAGE_MAPLE_TMPDIR='/tmp'
$ sage
Yes, if you don't specify the tmpdir, then this message gets printed (this was already there when I looked at the code). You can specify the tmpdir either in the environment (as you do) or from within the set_server_and_command method.
Replying to @mmasdeu:
Replying to @videlec:
An annoying feature
sage: magma.set_<TAB> Creating list of all Magma intrinsics for use in tab completion. ... Scanning Magma types ...
This only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in .sage//magma_intrinsic_cache.sobj
(with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)
Sided remark: there is a .format()
trick to deal with curly brackets. Double brackets are transformed into one
sage: 'SAGE_{{{{}}}}_{{}}_{}'.format('one').format('two').format('three')
'SAGE_three_two_one'
sage: '{{{}}}'.format(1)
'{1}'
As you moved the code of magma_version
into the class, you should deprecate it (since it is in the global namespace).
In the Magma
constructor there is the default option command = 'magma'
which prevents using the environment variable SAGE_MAGMA_COMMAND
.
Setting the default to None
is incompatible with
if not user_config:
command += ' -n'
which results in
TypeError: unsupported operand type(s) for +=: 'NoneType' and 'str'
Branch pushed to git repo; I updated commit sha1. New commits:
3197787 | Fixed user_config and environment variable clash. |
I fixed it by (also) reading the environment variable in magma.py. Each interface can do something similar if needed.
Replying to @videlec:
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in
.sage//magma_intrinsic_cache.sobj
(with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)
No, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.
Branch pushed to git repo; I updated commit sha1. New commits:
ce26d46 | Removed command_args parameter from one place. |
Replying to @nbruin:
Replying to @videlec:
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in
.sage//magma_intrinsic_cache.sobj
(with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)No, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.
I was not complaining about the location, just about potential version clashes. However, this seems to be completely broken (at least with magma 2.11.13 on my computer).
Marc, you did not address [comment:29], see Deprecation in the developer guide.
Branch pushed to git repo; I updated commit sha1. New commits:
c079a14 | Deprecated and removed from global namesplace the function magma_version. |
I have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!
Replying to @mmasdeu:
I have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!
Nope. There is another ticket which actually tracks the issue. This is a problem with the way Sage uses Ipython.
Technically, you deprecated the import in the global namespace but not the function in the module. In other words, this does not raise a warning as it should
sage: sage.interfaces.magma.magma_version()
(because in the long run, we should just get rid of magma_version
). Would be better to simply deprecate magma_version
and not modify all.py
using
def magma_version():
from sage.misc.superseded import deprecation
deprecation(20388, 'whatever')
return magma.version()
Other than that, everything looks good to me. Nils?
I have frequently encountered computer setups where one has access to a Sage installation locally, but Magma is only installed in a remote machine (say with address 'remote'). Typically one either: 1) Logs into 'remote' via ssh and does some computation in Magma. 2) uses the ssh command
to pretend having magma installed locally.
Actually Sage has functionality to use (2) in its interface with Magma, but it is currently broken and not advertised. It presumes that 'remote' also has sage installed (in fact it presumes that sage-native-execute is in the default PATH), for example.
This ticket fixes this, and makes a command like
work, and provide the user with an interface to a particular version of Magma installed in 'remote'.
Component: interfaces: optional
Keywords: magma, remote
Author: Marc Masdeu
Branch/Commit:
f499648
Reviewer: Nils Bruin, Vincent Delecroix
Issue created by migration from https://trac.sagemath.org/ticket/20388