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

Implement set and get nthreads for magma interface #26235

Closed
rusydi opened this issue Sep 10, 2018 · 13 comments
Closed

Implement set and get nthreads for magma interface #26235

rusydi opened this issue Sep 10, 2018 · 13 comments

Comments

@rusydi
Copy link
Contributor

rusydi commented Sep 10, 2018

This ticket implements the method to set and get the number of threads used to run some parallelized algorithms in Magma (e.g. Groebner bases).

CC: @slel

Component: interfaces: optional

Keywords: magma, threads

Author: Rusydi H. Makarim

Branch/Commit: 5860e68

Reviewer: Travis Scrimshaw

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

@rusydi rusydi added this to the sage-8.4 milestone Sep 10, 2018
@rusydi
Copy link
Contributor Author

rusydi commented Sep 10, 2018

Branch: u/ruhm/magma_threads

@rusydi

This comment has been minimized.

@rusydi
Copy link
Contributor Author

rusydi commented Sep 10, 2018

Author: Rusydi H. Makarim

@rusydi
Copy link
Contributor Author

rusydi commented Sep 10, 2018

New commits:

5860e68initial commit

@rusydi
Copy link
Contributor Author

rusydi commented Sep 10, 2018

Commit: 5860e68

@rusydi
Copy link
Contributor Author

rusydi commented Sep 10, 2018

Changed keywords from none to magma, threads

@tscrim
Copy link
Collaborator

tscrim commented Sep 10, 2018

comment:3

LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Sep 10, 2018

Reviewer: Travis Scrimshaw

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Sep 11, 2018

comment:4

It seems useless clutter to have each time two methods for the same thing.

The current proposition for this ticket is:

    def set_nthreads(self, n):
        """
        Set the number of threads used for parallelized algorithms in Magma.

        INPUT:

        - ``n`` - number of threads

        EXAMPLES::

            sage: magma.set_nthreads(2)                #optional - magma
            sage: magma.get_nthreads()                 #optional - magma
            2
        """
        self.SetNthreads(n)

    def SetNthreads(self, n):
        """
        Set the number of threads used for parallelized algorithms in Magma.

        INPUT:

        - ``n`` - number of threads

        .. note::

           This method is provided to be consistent with the Magma
           naming convention.

        EXAMPLES::

            sage: magma.SetNthreads(2)                 #optional - magma
            sage: magma.GetNthreads()                  #optional - magma
            2
        """
        self.eval('SetNthreads(%d)' % (n))

    def get_nthreads(self):
        """
        Get the number of threads used in Magma.

        EXAMPLES::

            sage: magma.set_nthreads(2)                #optional - magma
            sage: magma.get_nthreads()                 #optional - magma
            2
        """
        return self.GetNthreads()

    def GetNthreads(self):
        """
        Get the number of threads used in Magma.

        .. note::

           This method is provided to be consistent with the Magma
           naming convention.

        EXAMPLES::

            sage: magma.SetNthreads(2)                 #optional - magma
            sage: magma.GetNthreads()                  #optional - magma
            2
        """
        return int(self.eval('GetNthreads()'))

Among existing methods for the Magma class, one can find

  • four examples with only Python-style names; they call the corresponding
    Magma function using self.eval(...):
    def set_seed(self, seed=None):
        self.eval('SetSeed(%d)' % seed)

    def cputime(self, t=None):
        if t:
            return float(self.eval('Cputime(%s)' % t))
        else:
            return float(self.eval('Cputime()'))

    def chdir(self, dir):
        self.eval('ChangeDirectory("%s")' % dir, strip=False)

    def version(self):
        t = tuple([int(n) for n in self.eval('GetVersion()').split()])
        return t, 'V%s.%s-%s' % t
  • two examples with python-style names, which work like the four above,
    and are followed by synonyms with magma-style names:
    def attach(self, filename):
        self.eval('Attach("%s")' % filename)

    Attach = attach

    def attach_spec(self, filename):
        s = self.eval('AttachSpec("%s")' % filename)
        if s:
            raise RuntimeError(s.strip())

    AttachSpec = attach_spec
  • two examples where there is each time two methods, with the two naming styles,
    one calling the other, much like the current proposition in this ticket:
    def set_verbose(self, type, level):
        self.SetVerbose(type, level)

    def SetVerbose(self, type, level):
        if level < 0:
            raise TypeError("level must be >= 0")
        self.eval('SetVerbose("%s",%d)' % (type, level))

    def get_verbose(self, type):
        return self.GetVerbose(type)

    def GetVerbose(self, type):
        return int(self.eval('GetVerbose("%s")' % type))

For the current ticket, I would suggest using synonyms rather than
defining two separate methods each time:

    def set_nthreads(self, n):
        """
        Set the number of threads used for parallelized algorithms in Magma.

        INPUT:

        - ``n`` - number of threads

        EXAMPLES::

            sage: magma.set_nthreads(2)                #optional - magma
            sage: magma.get_nthreads()                 #optional - magma
            2
        """
        self.eval('SetNthreads(%d)' % (n))

    SetNthreads = set_nthreads

    def get_nthreads(self):
        """
        Get the number of threads used in Magma.

        EXAMPLES::

            sage: magma.set_nthreads(2)                #optional - magma
            sage: magma.get_nthreads()                 #optional - magma
            2
        """
        return int(self.eval('GetNthreads()'))

    GetNthreads = get_nthreads

And I would suggest taking the opportunity to do the same for
set_verbose/SetVerbose and get_verbose/GetVerbose.

Or... this cleaning-up can all be done in a follow-up ticket --- possibly
a better idea, especially since this ticket already has positive_review.

@slel slel changed the title implement set and get nthreads for magma interface Implement set and get nthreads for magma interface Sep 11, 2018
@rusydi
Copy link
Contributor Author

rusydi commented Sep 12, 2018

comment:5

Thanks for the remark. I will create a new ticket to deal with the cleaning up.

@rusydi
Copy link
Contributor Author

rusydi commented Sep 12, 2018

comment:6

The ticket is #26259

@vbraun
Copy link
Member

vbraun commented Sep 12, 2018

Changed branch from u/ruhm/magma_threads to 5860e68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants