Fix the Magma interface to work with remote installations

[mmasdeu]

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

    $> ssh -t remote 'magma'

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

    sage: magma = Magma(server = 'remote', command = 'magma-myversion')

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

[mmasdeu]

Branch: u/mmasdeu/20388

[mmasdeu]

New commits:

8cf2e2a	Fixed Magma interface to work with remote server.
5fb6df4	Fixed spacing.
ec0a724	Added message to help in discovering the feature.

[mmasdeu]

Commit: ec0a724

[videlec]

comment:2

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.

[mmasdeu]

comment:3

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.

[videlec]

comment:4

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.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

da0c8a1

Fixed two functions that were using the default magma.

[sagetrac-git]

Changed commit from ec0a724 to da0c8a1

[mmasdeu]

comment:6

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

[nbruin]

comment:7

A few things you might want to look at:

• sage's pexpect interfaces have an option to use files for transfer if the input is large. That's going it be an issue, because in your scenario you wouldn't want to assume sage and (remote) magma have access to a shared file system. So you might have to turn this capability off.
• You're now assuming that the remote magma user has permission to do an "scp" to the remote server. Indeed, the way we currently have written 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.

[videlec]

comment:8

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

[nbruin]

comment:9

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)

[mmasdeu]

comment:10

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...

[videlec]

comment:11

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.

[mmasdeu]

comment:12

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?

[videlec]

comment:13

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.

[nbruin]

comment:14

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.

[videlec]

comment:15

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

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.

[mmasdeu]

Changed branch from u/mmasdeu/20388 to u/mmasdeu/20388-fix

[mmasdeu]

New commits:

d4c1650	Disallowed file transfers when using remote Magma. Added error checking
d85b390	Added possibility of changing Magma defaults via environment variables.
a8ee09a	Added functionality in expect.py to set the server and command.

[mmasdeu]

Changed commit from da0c8a1 to a8ee09a

[videlec]

comment:17

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

[videlec]

comment:18

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.

[videlec]

comment:19

An annoying feature

sage: magma.set_<TAB>
Creating list of all Magma intrinsics for use in tab completion.
...
Scanning Magma types ...

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

8a28c1a

Added examples. Moved environment variables to expect.

[videlec]

comment:31

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'

[sagetrac-git]

Changed commit from 8a28c1a to 3197787

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

3197787

Fixed user_config and environment variable clash.

[mmasdeu]

comment:33

I fixed it by (also) reading the environment variable in magma.py. Each interface can do something similar if needed.

[nbruin]

comment:34

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.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

ce26d46

Removed command_args parameter from one place.

[sagetrac-git]

Changed commit from 3197787 to ce26d46

[videlec]

comment:37

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

[videlec]

comment:38

Marc, you did not address [comment:29], see Deprecation in the developer guide.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c079a14

Deprecated and removed from global namesplace the function magma_version.

[sagetrac-git]

Changed commit from ce26d46 to c079a14

[mmasdeu]

comment:40

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!

[videlec]

comment:41

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.

[videlec]

comment:42

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?

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

6c7236e	Revert "Deprecated and removed from global namesplace the function magma_version."
f499648	Deprecated magma_version instead of its importing.

[sagetrac-git]

Changed commit from c079a14 to f499648

[videlec]

comment:44

This good for me, only needs Nils agreement. Thanks for the fix. Possible follow-up: #13885

[nbruin]

comment:45

I don't have an axe to grind on this. My general impression is that the proposal here should improve the status quo.

[videlec]

Reviewer: Nils Bruin, Vincent Delecroix

[vbraun]

Changed branch from u/mmasdeu/20388-fix to f499648