scipopt / scip

SCIP - Solving Constraint Integer Programs
Other
393 stars 63 forks source link

Bliss API Compatability: `AbstractGraph::find_automorphisms`; outdated signature #16

Closed ndrwnaguib closed 2 years ago

ndrwnaguib commented 2 years ago

When trying to compile SCIP from source, the build fails with

error: cannot convert ‘void(void*, unsigned int, const unsigned int*)’ to ‘const std::function<void(unsigned int, const unsigned int*)>&

in https://github.com/scipopt/scip/blob/07dfd86f87f15f8d31eee700f603283bfb6ab357/src/symmetry/compute_symmetry_bliss.cpp#L1087

I looked up the reference, seems like Bliss has changed the signature (user_param is no longer accepted as a param), Is SCIP using a forked version of the API?

I was able to surpass the error by rewriting the blisshook to

    auto blisshook = [&data](
      unsigned int          n,                  /**< size of aut vector */
      const unsigned int*   aut                 /**< automorphism */
      )
    {
      assert( aut != NULL );

      assert( data.scip != NULL );
      assert( data.npermvars < (int) n );
      assert( data.maxgenerators >= 0);

      /* make sure we do not generate more that maxgenerators many permutations, if the limit in bliss is not available */
      if ( data.maxgenerators != 0 && data.nperms >= data.maxgenerators )
          return;

      /* copy first part of automorphism */
      bool isIdentity = true;
      int* p = 0;
      if ( SCIPallocBlockMemoryArray(data.scip, &p, data.npermvars) != SCIP_OKAY )
          return;

      for (int j = 0; j < data.npermvars; ++j)
      {
          /* convert index of variable-level 0-nodes to variable indices */
          p[j] = (int) aut[j];
          if ( p[j] != j )
            isIdentity = false;
      }

      /* ignore trivial generators, i.e. generators that only permute the constraints */
      if ( isIdentity )
      {
          SCIPfreeBlockMemoryArray(data.scip, &p, data.npermvars);
          return;
      }

      /* check whether we should allocate space for perms */
      if ( data.nmaxperms <= 0 )
      {
          if ( data.maxgenerators == 0 )
            data.nmaxperms = 100;   /* seems to cover many cases */
          else
            data.nmaxperms = data.maxgenerators;

          if ( SCIPallocBlockMemoryArray(data.scip, &data.perms, data.nmaxperms) != SCIP_OKAY )
            return;
      }
      else if ( data.nperms >= data.nmaxperms )    /* check whether we need to resize */
      {
          int newsize = SCIPcalcMemGrowSize(data.scip, data.nperms + 1);
          assert( newsize >= data.nperms );
          assert( data.maxgenerators == 0 );

          if ( SCIPreallocBlockMemoryArray(data.scip, &data.perms, data.nmaxperms, newsize) != SCIP_OKAY )
            return;

          data.nmaxperms = newsize;
      }

      data.perms[data.nperms++] = p;
    };

and the call to

   G.find_automorphisms(stats, blisshook);

It is now working as expected, if you think that's a correct fix, I can prepare a patch :).

Thanks.

ndrwnaguib commented 2 years ago

The same incompatibility shows in gcg/src/dec_isomorph.cpp and gcg/src/bliss_automorph.cpp

fschloesser commented 2 years ago

Dear @ndrwnaguib Using the latest version 0.77 from https://users.aalto.fi/~tjunttil/bliss/download.html the error does not appear in my compilation. The downloadable packages are built with the github version of bliss https://github.com/ds4dm/Bliss mentioned in the installation section https://www.scipopt.org/doc-8.0.0/html/md_INSTALL.php. This does also not fail in compilation for me. What are you doing differently than me? Best, Franzi

ndrwnaguib commented 2 years ago

Hi Franzi; I am also using bliss 0.77, the second rc though.

Name            : bliss
Version         : 0.77-2
Description     : A library for computing automorphism groups and canonical forms of graphs
Architecture    : x86_64
URL             : https://users.aalto.fi/~tjunttil/bliss/
Licenses        : GPL3
Groups          : None
Provides        : None
Depends On      : gmp
Optional Deps   : None
Required By     : None
Optional For    : None
Conflicts With  : None
Replaces        : None
Installed Size  : 362.86 KiB
Install Reason  : Installed as a dependency for another package
Install Script  : No
Validated By    : Signature

I'm not sure why we have different results; especially that the change took effect starting from version 0.75 (according to the release notes)

fschloesser commented 2 years ago

Hey @ndrwnaguib How did you install bliss, and are the include files that cmake finds the correct ones or do you possibly have old include files lying around somewhere on your system? To me, the signature of the find_automorphisms call looks correct in the scip code, as is takes the three arguments mentioned in the documentation. But @pfetsch is the expert here. Best, Franzi

ndrwnaguib commented 2 years ago

Franzi, thanks for the follow up; I installed it from the Arch Linux official repositories.

To me, the signature of the find_automorphisms call looks correct in the scip code, as is takes the three arguments mentioned in the documentation.

I think the documentation lists the third parameter as std::function<bool()> type though, with the following description:

If the terminate function argument is given, it is called in each search tree node: if the function returns true, then the search is terminated and thus not all the automorphisms may have been generated. The terminate function may be used to limit the time spent in bliss in case the graph is too difficult under the available time constraints. If used, keep the function simple to evaluate so that it does not consume too much time.

In the referenced SCIP line, it's being used as (void*) (and statically casted to BLISS_data later on); the compilation fails to convert SCIP call to the function to conform with the signature of my bliss implementation however.

pfetsch commented 2 years ago

As far as I can see, the code takes the new interface into account, see the part starting with

#if BLISS_VERSION_MAJOR >= 1 || BLISS_VERSION_MINOR >= 76

in scip/compute_symmetry_bliss.cpp. The corresponding call in this case is

G.find_automorphisms(stats, reportglue, term);

where term is the callback function.

Can you please check whether the define is true in your case (it should be with Bliss version 0.77).

ndrwnaguib commented 2 years ago

@pfetsch, thanks for noting that, it is not there. I was building 7.0.3 instead of 8.0.0. The latest SCIP version should be working however considering the lines you referenced; thank you.