Opened 3 years ago

Closed 3 years ago

#26235 closed enhancement (fixed)

Implement set and get nthreads for magma interface

Reported by: ruhm Owned by:
Priority: minor Milestone: sage-8.4
Component: interfaces: optional Keywords: magma, threads
Cc: slelievre Merged in:
Authors: Rusydi H. Makarim Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 5860e68 (Commits, GitHub, GitLab) Commit: 5860e688511b6a4e690000e336cd73177822f0e2
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

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

Change History (7)

comment:1 Changed 3 years ago by ruhm

  • Branch set to u/ruhm/magma_threads

comment:2 Changed 3 years ago by ruhm

  • Authors set to Rusydi H. Makarim
  • Commit set to 5860e688511b6a4e690000e336cd73177822f0e2
  • Component changed from PLEASE CHANGE to interfaces: optional
  • Description modified (diff)
  • Keywords magma threads added
  • Priority changed from major to minor
  • Status changed from new to needs_review
  • Type changed from PLEASE CHANGE to enhancement

New commits:

5860e68initial commit

comment:3 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:4 Changed 3 years ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Summary changed from implement set and get nthreads for magma interface to Implement set and get nthreads for magma interface

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.

comment:5 Changed 3 years ago by ruhm

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

comment:6 Changed 3 years ago by ruhm

The ticket is #26259

comment:7 Changed 3 years ago by vbraun

  • Branch changed from u/ruhm/magma_threads to 5860e688511b6a4e690000e336cd73177822f0e2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.