sa2c / HiRep

HiRep repository
GNU General Public License v2.0
0 stars 4 forks source link

configuration converter with quaternions #1

Open dvadacchino opened 3 years ago

dvadacchino commented 3 years ago

It is not possible to use the Hirep builtin converter to convert quaternion configurations to matrix configurations.

Trying to compile the converter in Hirep/Converter with N=2 and quaternions set in the Make/MkFlags file results in several compilation errors, starting with lines 211 and 399 of file Hirep/Converter/archive_eolexi.c ( See below ).

My guess is that these files need a new preprocessor if conditional for the case of quaternions.

======================================

$ cd Hirep/Converter $ make . . .

Compiling [%/Converter/archive_eolexi.o]...archive_eolexi.c: In function ‘read_gauge_field_eolexi_BE’: archive_eolexi.c:211:70: error: request for member ‘re’ in something not a structure or union lprintf("IO",20,"(%.2f , %.2f) ",pu_gauge(ix,mu)->c[iNG+j].re, pu_gauge(ix,mu)->c[iNG+j].im); ^ archive_eolexi.c:211:101: error: request for member ‘im’ in something not a structure or union lprintf("IO",20,"(%.2f , %.2f) ",pu_gauge(ix,mu)->c[iNG+j].re, pu_gauge(ix,mu)->c[iNG+j].im); ^ archive_eolexi.c: In function ‘read_gauge_field_eolexi_LE’: archive_eolexi.c:399:70: error: request for member ‘re’ in something not a structure or union lprintf("IO",20,"(%.2f , %.2f) ",pu_gauge(ix,mu)->c[iNG+j].re,pu_gauge(ix,mu)->c[iNG+j].im); ^ archive_eolexi.c:399:100: error: request for member ‘im’ in something not a structure or union lprintf("IO",20,"(%.2f , %.2f) ",pu_gauge(ix,mu)->c[iNG+j].re,pu_gauge(ix,mu)->c[iNG+j].im);

$

Here is the Make/MkFlags file

======================================

NG = 2 REPR = REPR_FUNDAMENTAL

REPR = REPR_FUNDAMENTAL

REPR = REPR_SYMMETRIC

REPR = REPR_ANTISYMMETRIC

REPR = REPR_ADJOINT

CHOICES ARE GAUGE_SUN, GAUGE_SON AND GAUGE_SPN

GAUGE_GROUP = GAUGE_SUN

T => PERIODIC, ANTIPERIODIC, OPEN, THETA

X => PERIODIC, ANTIPERIODIC, THETA

Y => PERIODIC, ANTIPERIODIC, THETA

Z => PERIODIC, ANTIPERIODIC, THETA

MACRO += -DBC_T_THETA

MACRO += -DBC_T_PERIODIC MACRO += -DBC_X_PERIODIC MACRO += -DBC_Y_PERIODIC MACRO += -DBC_Z_PERIODIC

MACRO += -DBC_XYZ_TWISTED

MACRO += -DHALFBG_SF

MACRO += -DBASIC_SF

MACRO += -DROTATED_SF

MACRO += -DUPDATE_EO

MACRO += -DWITH_MPI

MACRO += -DWITH_UMBRELLA

MACRO += -DWITH_QUATERNIONS

MACRO += -DNDEBUG

MACRO += -DCHECK_SPINOR_MATCHING

MACRO += -DMPI_TIMING

MACRO += -DIO_FLUSH

MACRO += -DWITH_CLOVER

MACRO += -DUNROLL_GROUP_REPRESENT

MACRO += -DTIMING

MACRO += -DTIMING_WITH_BARRIERS

MACRO += -DAMALLOC_MEASURE

MACRO += -DADAPTIVE

Compiler

CC = mpicc

CC = gcc

CFLAGS = -Wall -std=c99 -O2 -fomit-frame-pointer -msse -msse2

CFLAGS = -Wall -std=c99 -g -O3 LDFLAGS =

========================================

edbennett commented 3 years ago

The converter hasn't been updated in the upstream code since the change to C99 complex numbers. I took a look a while back at making it work, but there were recent changes in the process initialisation (for example, the -i option now must specify a text input file, not a configuration) that meant that even with C99 complexes fixed, things still failed, and it was non-trivial to adjust the setup code.

Now that the setup code is a little more settled, and other measurement tools have been updated to work with it (one must now specify the configuration or list file as a parameter in an input file), this should be more achievable.

Turns out that the problems I was looking at have already been fixed (over a year ago) by Antonio. Apologies for sharing out-of-date information!

mmesiti commented 3 years ago

On branch spn-swansea-merge (0e91a96623a874963ec9a02e10ca1e53ecdad765) the im/re issue has been corrected, AFAIK. Still, there are other issues - namely, read_gauge_field_su2q and write_gauge_field_su2q are undefined for NG==2 and -DWITH_QUATERNIONS. The following diff

diff --git a/LibHR/IO/archive_su2quat.c b/LibHR/IO/archive_su2quat.c
index 6597e3d..73cde00 100644
--- a/LibHR/IO/archive_su2quat.c
+++ b/LibHR/IO/archive_su2quat.c
@@ -23,7 +23,7 @@
 #include "utils.h"
 #include "ranlux.h"

-#if NG == 2 && !defined(WITH_QUATERNIONS)
+#if NG == 2 && defined(WITH_QUATERNIONS)

 #ifdef NDEBUG
 #define MPIRET(type)

seems to solve the compilation issue, but the fix seems to easy that it looks fishy.

edbennett commented 3 years ago

This behaviour (without the diff that @mmesiti proposes) is correct. When compiling with -DWITH_QUATERNIONS, then read_gauge_field and write_gauge_field already read and write gauge fields in the quaternion format, so the separate su2q functions are redundant. In order to convert from matrix to su2q and back, you need to compile without -DWITH_QUATERNIONS, so that the matrix functions are available, in which case the _su2q functions are provided to handle the su2q format.

mmesiti commented 3 years ago

So, is the inconsistency instead at Converter/converter.c:71-73? If so, then this other patch works:

diff --git a/Converter/converter.c b/Converter/converter.c
index ae5d9fa..ba731c3 100644
--- a/Converter/converter.c
+++ b/Converter/converter.c
@@ -68,7 +68,7 @@ format_type format[nformats] = {
     {.name = "eolexi:le", .read = read_gauge_field_eolexi_LE, .write = write_gauge_field_eolexi_LE},
     {.name = "mpieo:le", .read = read_gauge_field_mpieo_LE, .write = write_gauge_field_mpieo_LE},
     {.name = "fortran", .read = read_gauge_field_fortran, .write = NULL},
-#ifdef WITH_QUATERNIONS
+#ifndef WITH_QUATERNIONS
     {.name = "su2q", .read = read_gauge_field_su2q, .write = write_gauge_field_su2q},
 #endif
 #ifdef GAUGE_SPN

The fix would also be compatible with

  #ifdef WITH_QUATERNIONS                                                                      
    #define nformats 10                                                                                                                                                         
  #else                                                                                        
    #define nformats 11                                                                                                                                                         
  #endif

some lines above that, in Converter/converter.c. The #ifdef in the format[] definition and the defintion of nformats do not seem consistent (this was true also at commit 74aa3bb, for example).

edbennett commented 3 years ago

Yes, I think that's where the bug is (and now you've posted the diff I remember making that change previously).

edbennett commented 3 years ago

@dvadacchino I've just pushed 022f909 which I believe fixes this. Can you test, please?

Compile with -DWITH_QUATERNIONS disabled, and then use something like mkdir matrix_cfgs && ./converter -i some_quaternion_configuration su2q -d matrix_cfgs mpieo