Keep Bridge state across several calls?

[valassi]

Hi @roiser, @oliviermattelaer

After our chat yesterday at some point I realised there may be one issue with the current Bridge interface: we keep creating a new object

madgraph4gpu/epoch2/fortran-cuda/pp_ttx/SubProcesses/fbridge.cu

Line 30 in 774a900

Bridge<double> b = Bridge<double>(16, 4, 4, 4, 16);

madgraph4gpu/epoch2/fortran-cuda/pp_ttx/SubProcesses/bridge.h

Line 115 in 774a900

process.initProc("../../Cards/param_card.dat");

This would not leak memory because it is stack allocated, however it has two issues

• Most importantly, it would calculate good helicities over and over, at each call
• I also have the impression that it would read the parameter file every time
• In addition, it is a bit silly to create a Bridge and delete it at every iteration of matrix element calculations

Since it is a bad idea to keep static instances in the cuda/c++ module, I think that it is somehow unavoidable to pass the knowledge of the Bridge C++ pointer to Fortran. I made a couple of quick tests and this looks quite easy.

For instance, look at this code

File Bridge.h

#ifndef BRIDGE_H 
#define BRIDGE_H 1
#include <iostream>
class Bridge {
public:
  Bridge() : m_init( false ){ std::cout << "Creating Bridge " << this << std::endl; }
  virtual ~Bridge(){ std::cout << "Deleting Bridge " << this << std::endl; }  
  void doStuff( const double x ){
    if ( ! m_init ){ std::cout << "Initialising" << std::endl; m_init = true; }
    std::cout << "Doing stuff " << x << std::endl;
  }  
private:
  bool m_init;
};
#endif // BRIDGE_H

File prog.f

c See https://stackoverflow.com/a/24762346
c (See also https://docs.oracle.com/cd/E19059-01/stud.9/817-6694/11_cfort.html)
c (See also https://stackoverflow.com/a/29154061)
      PROGRAM FPROG
      IMPLICIT NONE
      INTEGER*8 BRIDGE
      INTEGER ILOOP, NLOOP
      DOUBLE PRECISION XLOOP
      CALL FCREATEBRIDGE( BRIDGE )
      NLOOP = 5
      DO ILOOP = 1, NLOOP
        XLOOP = 1.1 * ILOOP
c       WRITE(*,*) "XLOOP =", XLOOP
        CALL FDOSTUFF( BRIDGE, XLOOP )
      END DO
      CALL FDELETEBRIDGE( BRIDGE )
      END PROGRAM FPROG

File bridge.cpp

#include "Bridge.h"
Bridge* createBridge(){ return new Bridge(); }
void doStuff( Bridge* pbridge, double x ){ pbridge->doStuff( x ); }
void deleteBridge( Bridge* pbridge ){ delete pbridge; }
extern "C"
{
  void fcreatebridge_( Bridge** ppbridge ){ *ppbridge = createBridge(); }  
  void fdostuff_( Bridge** ppbridge, double* px ){ doStuff( *ppbridge, *px ); }
  void fdeletebridge_( Bridge** ppbridge ){ deleteBridge( *ppbridge ); }
}

Building and running:

> g++ -o bridge.o -c bridge.cpp; gfortran prog.f bridge.o -lstdc++
> ./a.out 
Creating Bridge 0x1197290
Initialising
Doing stuff 1.1
Doing stuff 2.2
Doing stuff 3.3
Doing stuff 4.4
Doing stuff 5.5
Deleting Bridge 0x1197290

There are other alternatives possible to avoid the FCREATEBRIDGE call, but then essentially one leaks the Bridge at the end of the program (unless overcomplicating it). In any case I think that getting the notion of a pointer (here as INTEGER*8 for an x64 double precision memory system) is kind of unavoidable.

This approach would also have other advantages, e.g. it allows the Fortran to cleanly pass any configuration parameters to teh Bridge after construction and before the first use, if we choose to do so. For instance one may pass as a single paramater the choice whether the ME calculation is on the CPU or the GPU. Or one can pass the number of blocks and threads in the GPU grid (or eventually more exotic configurations for heterogeneous computing).

Would you agree to this type of changes? Olivier do you see a problem implementing this in the Fortran? You essentially only need to declare an extra INTEGER*8 and then be sure you have an initialization call in that Frortran routine ("if first then create the bridge" will do, even if it leaks the memory at the end, but it is good enough for a beginning).

Thanks!
Andrea

[valassi]

@jtchilders , @nscottnichols , as I just briefly mentioned via email, I think this kind of design should also allow you to cleanly call an initialise and a finalise function for kokkos, within the fcreatebridge and fdeletebridge functions (or rather within the Bridge ctor/dtor which arecalled from those). Does this make sense or would you have other suggestions? Thanks

[nscottnichols]

This kind of bridge makes a lot of sense and works for me.

[roiser]

Hi, I'm a bit wary about having 3 calls across the languages. I'd say this is our weakest point in the architecture, calling just c synbols. I had an idea on how to avoid it and woukd try it next week. Bon week-end, Stefan

[valassi]

Hi, thanks @nscottnichols @jtchilders and @roiser for the feedback.

Stefan, I do not see any problem with having three calls across languages. If we need initialization, computations and finalizations, that's the only way I see, or at least the cleanest.

As I said in my first post, there are other ways with a singe call, but they leak resources, and in any case I find them less clean. I had not included the code, doing this now:

File bridg2.cpp

#include "Bridge.h"
void doStuff( Bridge*& bridge, double x ){ if ( !bridge ) bridge = new Bridge(); bridge->doStuff( x ); }
extern "C"
{
  void fdostuff_( Bridge** bridge, double* x ){ doStuff( *bridge, *x ); }
}

File prog2.f

      PROGRAM FPROG
      IMPLICIT NONE
      INTEGER*8 BRIDGE/0/
      INTEGER ILOOP, NLOOP
      DOUBLE PRECISION XLOOP
      NLOOP = 5
      DO ILOOP = 1, NLOOP
        XLOOP = 1.1 * ILOOP
c       WRITE(*,*) "XLOOP =", XLOOP
        CALL FDOSTUFF( BRIDGE, XLOOP )
      END DO
      END PROGRAM FPROG

Building and running

> g++ -o bridge2.o -c bridge2.cpp; gfortran prog2.f bridge2.o -lstdc++
> ./a.out 
Creating Bridge 0x2335290
Initialising
Doing stuff 1.1
Doing stuff 2.2
Doing stuff 3.3
Doing stuff 4.4
Doing stuff 5.5

It works, but the Bridge is not deleted at the end. Note in any case that you still need some sort of initialisation (BRIDGE/0/) because in any case on the C++ side you must have a single instance of the relevant class(es). Again, if you want to avoid statics, allow multithreading, have a single helicity decision and a single reading from parameter file, I would go this way.

Anyway, if you have an alternative try it out and let me know!
Thanks
Andrea

[valassi]

PS @oliviermattelaer what's your take?

[roiser]

@valassi concerning the calls, sorry I didn't explain myself enough. The problem I see is that we are just calling c symbols without any protection for what concerns the signature of the function, this looks very fragile to me. Therefore I would suggest to reduce these numbers of calls to a minimum (one if possible). I'll try sth and come back to you later.

[valassi]

Hi Stefan,

I do not think it's a good idea to avoid explicit create/delete calls, for the reasons above (mainly, resource leaks due to missing delete).

I understand your concern about the missing signature check instead. I just had a quick look, fortran provides an easy way to do this via the INTERFACE statement.

This is yest another version of the code (with three calls). Keep the same Bridge.h and bridge.cpp as above from the first post, then instead use these:

File bridge_interface.inc

      INTERFACE
         SUBROUTINE FCREATEBRIDGE(PBRIDGE)
         INTEGER*8 PBRIDGE
         END SUBROUTINE FCREATEBRIDGE
         SUBROUTINE FDOSTUFF(PBRIDGE, XARG)
         INTEGER*8 PBRIDGE
         DOUBLE PRECISION XARG
         END SUBROUTINE FDOSTUFF
         SUBROUTINE FDELETEBRIDGE(PBRIDGE)
         INTEGER*8 PBRIDGE
         END SUBROUTINE FDELETEBRIDGE
      END INTERFACE

File fprog_include.f

      PROGRAM FPROG
      IMPLICIT NONE
      INCLUDE 'bridge_interface.inc'
      INTEGER*8 BRIDGE
      INTEGER ILOOP, NLOOP
      DOUBLE PRECISION XLOOP
      CALL FCREATEBRIDGE( BRIDGE )
      NLOOP = 5
      DO ILOOP = 1, NLOOP
        XLOOP = 1.1 * ILOOP
c       WRITE(*,*) "XLOOP =", XLOOP
        CALL FDOSTUFF( BRIDGE, XLOOP )
      END DO
      CALL FDELETEBRIDGE( BRIDGE )
      END PROGRAM FPROG

Build and run:

> g++ -o bridge.o -c bridge.cpp; gfortran prog_include.f bridge.o -lstdc++
> ./a.out 
Creating Bridge 0x15bc290
Initialising
Doing stuff 1.1
Doing stuff 2.2
Doing stuff 3.3
Doing stuff 4.4
Doing stuff 5.5
Deleting Bridge 0x15bc290

Would this be better? This can even be further refined using MODULE in Fortran, but that looks like an overkill to me.

Essentially what I propose is that also the bridge_interface.inc file would need to be created (and possibly tested?) from the cuda/c++. Essentially the cuda/c++ would then create BOTH a library (with the fcreatebridge, fdostuff and fdeletebridge symbols) AND an include file (declaring the interface for those symbols). The Fortran MadEvent would then simply include that inc file and link the library.

Let me know...
Thanks!
Andrea

[roiser]

Hi @valassi , to me it doesn't change a lot IIUC, we can still call whatever symbol we like on the C side and it doesn't need to match what we define in fortran. As said, I have an idea and will come back to this thread hopefully soon. best Stefan

[valassi]

Hi Stefan,

what I proposed constrains the prog.f (so MadEvent) to use exactly the interface defined in the bridge_interface.inc, otherwise it fails

File prog_include_wrong.f

      PROGRAM FPROG
      IMPLICIT NONE
      INCLUDE 'bridge_interface.inc'
      INTEGER*8 BRIDGE
      INTEGER ILOOP, NLOOP
c     DOUBLE PRECISION XLOOP
      REAL XLOOP
      CALL FCREATEBRIDGE( BRIDGE )
      NLOOP = 5
      DO ILOOP = 1, NLOOP
        XLOOP = 1.1 * ILOOP
c       WRITE(*,*) "XLOOP =", XLOOP
        CALL FDOSTUFF( BRIDGE, XLOOP )
      END DO
      CALL FDELETEBRIDGE( BRIDGE )
      END PROGRAM FPROG

Build fails

g++ -o bridge.o -c bridge.cpp; gfortran prog_include_wrong.f bridge.o -lstdc++
prog_include_wrong.f:16:72:

   16 |         CALL FDOSTUFF( BRIDGE, XLOOP )
      |                                                                        1
Error: Type mismatch in argument ‘xarg’ at (1); passed REAL(4) to REAL(8)

It does not force the bridge_interface.f to have the same interface as the C++ class however, you are right in that. This is why I suggest that also the inc file is created as part of the cuda/c++ library, it is the responsability of the library to keep the intenral consistency of the modules it offers with the Fortran interface it declares. To me that sounds enough.

As I said one can start using Fortran MODULEs, but I think that might become an overkill without much added value IIUC.

Anyway try out the other suggestion and let me know, no hurry!
Thanks
Andrea

[roiser]

Yes but what I mean is that the c function signature can be anything, that's problematic to my mind, give me a bit more time ...

[roiser]

Hi @valassi ,

here is an example on how it could be done. This should be thread safe and we would only need to call one bridge c symbol. All the initialisation/destruction can go into the constructor/destructor of the Bridge class and would be called once. It's a tiny bit safer and would keep the two worlds a bit better apart to my mind. The main() obviously will be the fortran part, but as I'm lazy keep all in one file for now.

Please let me know what you think

cheers
Stefan

#include <cstring>
#include <iostream>
#include <string>

class Bridge {
public:
  static Bridge &getBridge() {
    static Bridge b;
    return b;
  }

  void call_sequence(int *in, int *out) {
    int tmp = (*in) * (*in);
    memcpy(out, &tmp, sizeof(int));
  }

private:
  Bridge() { std::cout << "initialise Bridge" << std::endl; }

  ~Bridge() { std::cout << "delete Bridge" << std::endl; }

  Bridge(const Bridge &other);

  const Bridge &operator=(const Bridge &other);
};

void bridge(int *in, int *out) {
  Bridge &b = Bridge::getBridge();
  b.call_sequence(in, out);
}

extern "C" {
void fbridge(int *in, int *out) { bridge(in, out); }
}

int main() {
  int out = 0;
  for (int in = 0; in < 5; ++in) {
    bridge(&in, &out);
    std::cout << "bridge in:" << in << ", out: " << out << std::endl;
  }
  return 0;
}

runs with

initialise Bridge
bridge in:0, out: 0
bridge in:1, out: 1
bridge in:2, out: 4
bridge in:3, out: 9
bridge in:4, out: 16
delete Bridge

[valassi]

Hi Stefan,
what I do not like is the static Bridge...
Andrea

[valassi]

Sorry for the quick answer before, doing other things at the same time. I don't know, avoiding static is one of the things I said at the very beginning of this issue as being one goal. I agree that initially we will only have one bridge from Fortran, so we could actually have that static singleton in C++.

I just find it cleaner and leaving more future flexibility to let the Fortran know it is talking to one specific object. Anyway, initially maybe we can go for your static.

=> What do other people think? @oliviermattelaer especially, but also @nscottnichols and @jtchilders ?

Anyway, irrespective of whether we go for the static or an explicit instance, I would add a couple of comments:

• Now that I saw that it is easy, I would add this INTERFACE anyway. Would you not agree? You seemed concerned that you can call any interface from Fortran to C, using a static does not adfdress that issue.

• I would still keep in mind that this interface may be eventually extended. We may want to pass extra parameters from fortran to c++, eg the grid size or whatever we'll think about. Using a static allows that too (you have that singleton getBridge). But this means there may be more than one single call - to be declared in the interface.

• Along a similar line: I would say that we need at least a declaration of how many events are in the Fortran loop. Actually this needs to be declared when creating the Bridge as the C++ and CUDA buffers must be created when Bridge is created. My current bridge test from c++ does pass the number of events (and of gputhreads and gpublocks) through the Bridge constructor. To me this is one more reason why I would have an explicit createbridge call in the Fortran, with the correct number of events. Having a default somewhere sounds quite ugly (who decides the default? the c++ or the fortran? if it is the c++ you must expose it to the fortran, if it is the fortran you must expose it to the c++).

What do you think?

Thanks
Andrea

[roiser]

Hi @valassi

thanks, I think we are on the same page. Just for clarification, I am worried about calling the c symbols. What this snippet above does it not to avoid the risk but it reduces it as we call only one (for now). Yes in case we need more we can add but I'm even not sure that we need so much more. I'd say we cross the bridge when we get there ;-), if we shall really need it should be easy to add more stuff. Let's see what others think about it.

thanks
Stefan

[valassi]

I am finalising the new Bridge, as discussed.

For completeness, this is the proof of concept implementation.

Bridge3.h

#ifndef BRIDGE3_H
#define BRIDGE3_H 1
#include <iostream>
class Bridge
{
public:
  Bridge( const int nevt, const int np4 ) : m_iiter( 0 ), m_nevt( nevt ), m_np4( np4 )
  { std::cout << "Creating Bridge " << this << " with nevt=" << nevt << std::endl; }
  virtual ~Bridge(){ std::cout << "Deleting Bridge " << this << std::endl; }
  void sequence( const double* momenta, double* mes )
  {
    if ( m_iiter==0 ){ std::cout << std::endl << "Initialising" << std::endl << std::endl; }
    m_iiter++;
    for ( int ievt=0; ievt<m_nevt; ievt++ ) mes[ievt] = 0;
    for ( int ievt=0; ievt<m_nevt; ievt++ )
    { 
      std::cout << "Sequence #" << m_iiter << " ievt#" << ievt+1 << std::endl;
      for ( int ip4=0; ip4<m_np4; ip4++ )
      {
        mes[ievt] += momenta[ievt*m_np4 + ip4]; // c-array momenta[nevt][np4]
        std::cout << "Mom=" << momenta[ievt*m_np4 + ip4] << " MEs=" << mes[ievt] << std::endl;
      }
      std::cout << std::endl;
    }
  }
private:
  int m_iiter;
  int m_nevt;
  int m_np4;
};
#endif

bridge3.cpp

#include "Bridge3.h"
extern "C"
{
  void fbridgecreate_( Bridge** ppbridge, const int* pnevt, const int* pnp4 ){ *ppbridge = new Bridge( *pnevt, *pnp4 ); }
  void fbridgedelete_( Bridge** ppbridge ){ delete *ppbridge; }
  void fbridgesequence_( Bridge** ppbridge, const double* momenta, double* mes ){ (*ppbridge)->sequence( momenta, mes ); }
}

bridge3_interface.inc

      INTERFACE
         SUBROUTINE FBRIDGECREATE(PBRIDGE,NEVT,NP4)
         INTEGER*8 PBRIDGE
         INTEGER NEVT
         INTEGER NP4
         END SUBROUTINE FBRIDGECREATE
      END INTERFACE

      INTERFACE
         SUBROUTINE FBRIDGESEQUENCE(PBRIDGE,MOMENTA,MES)
         INTEGER*8 PBRIDGE
         DOUBLE PRECISION MOMENTA(*)
         DOUBLE PRECISION MES(*)
         END SUBROUTINE FBRIDGESEQUENCE
      END INTERFACE

      INTERFACE
         SUBROUTINE FBRIDGEDELETE(PBRIDGE)
         INTEGER*8 PBRIDGE
         END SUBROUTINE FBRIDGEDELETE
      END INTERFACE

prog3_include.f

      PROGRAM FPROG3
      IMPLICIT NONE
      INCLUDE 'bridge3_interface.inc'
      INTEGER*8 BRIDGE
      INTEGER NITER, NEVT, NP4
      PARAMETER(NITER=2, NEVT=3, NP4=4)
      INTEGER IITER, IEVT
      DOUBLE PRECISION MOMENTA(NP4,NEVT) ! c-array momenta[nevt][np4]
      DOUBLE PRECISION MES(NEVT)
      CALL FBRIDGECREATE(BRIDGE,NEVT,NP4)
      DO IEVT = 1, NEVT
        MOMENTA(1,IEVT) = 1.1 * IEVT
        MOMENTA(2,IEVT) = 1.01 * IEVT
        MOMENTA(3,IEVT) = 1.001 * IEVT
        MOMENTA(4,IEVT) = 1.0001 * IEVT
      END DO
      DO IITER = 1, NITER
        CALL FBRIDGESEQUENCE(BRIDGE,MOMENTA,MES)
        DO IEVT = 1, NEVT
          WRITE(6,*) 'MES',IEVT,MES(IEVT)
        END DO
        WRITE(6,*)
      END DO
      CALL FBRIDGEDELETE(BRIDGE)
      END PROGRAM FPROG3

Execution

[avalassi@itscrd70 gcc10.2/cvmfs] /data/avalassi/GPU2020/madgraph4gpuX/epochX/cudacpp/ee_mumu/SubProcesses/P1_Sigma_sm_epem_mupmum/FORTRAN3> g++ -o bridge3.o -c bridge3.cpp; gfortran prog3_include.f bridge3.o -lstdc++
[avalassi@itscrd70 gcc10.2/cvmfs] /data/avalassi/GPU2020/madgraph4gpuX/epochX/cudacpp/ee_mumu/SubProcesses/P1_Sigma_sm_epem_mupmum/FORTRAN3> ./a.out 
Creating Bridge 0x2125290 with nevt=3

Initialising

Sequence #1 ievt#1
Mom=1.1 MEs=1.1
Mom=1.01 MEs=2.11
Mom=1.001 MEs=3.111
Mom=1.0001 MEs=4.1111

Sequence #1 ievt#2
Mom=2.2 MEs=2.2
Mom=2.02 MEs=4.22
Mom=2.002 MEs=6.222
Mom=2.0002 MEs=8.2222

Sequence #1 ievt#3
Mom=3.3 MEs=3.3
Mom=3.03 MEs=6.33
Mom=3.003 MEs=9.333
Mom=3.0003 MEs=12.3333

 MES           1   4.1111000776290894     
 MES           2   8.2222001552581787     
 MES           3   12.333300352096558     

Sequence #2 ievt#1
Mom=1.1 MEs=1.1
Mom=1.01 MEs=2.11
Mom=1.001 MEs=3.111
Mom=1.0001 MEs=4.1111

Sequence #2 ievt#2
Mom=2.2 MEs=2.2
Mom=2.02 MEs=4.22
Mom=2.002 MEs=6.222
Mom=2.0002 MEs=8.2222

Sequence #2 ievt#3
Mom=3.3 MEs=3.3
Mom=3.03 MEs=6.33
Mom=3.003 MEs=9.333
Mom=3.0003 MEs=12.3333

 MES           1   4.1111000776290894     
 MES           2   8.2222001552581787     
 MES           3   12.333300352096558     

Deleting Bridge 0x2125290

[valassi]

I have completed a standalone Fortran test in PR #367. This is a Fortran program that fills a Fortran momenta array using a c++ sampler (common host + rambo sequence), and then computes MEs using a c++ or cuda bridge sequence. The results are the same in this Fortran program as they are in the standalone c++/cuda program.

[valassi]

I am about to merge PR #367 which implements all the points discuss in this issue #329. Closing.