Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#688 closed task (fixed)

Let user choose the I/O tool

Reported by: Ian Culverwell Owned by: Ian Culverwell
Priority: normal Milestone: 11.1
Component: ropp_io Version: 10.0
Keywords: Cc: olewis

Description

ROPP-10 beta reviewers Christian Marquardt and Leonid Butenko say:

If binaries are only built using the eccodes library (given that bufrdc hasn't
been maintained since a long time), the standard executables (e.g., eum2bufr) do
not even exist. One way around for a user is to manually create symbolic links
to the existing binaries after installation; this could be automated at
installation time. An alternative option could be to install a user-selectable
version under the default names. In case such behaviour is not implemented, the
READMEs and other documentation should explain the chosen approach and make
users aware that they need to manually add symbolic links or rename some
binaries when building ROPP against ectools.

I think the general idea of building as many tools as the dependencies allow (e.g. ropp2bufr_mobufr, ropp2bufr_ecbufr and ropp2bufr_eccodes, if possible), and then pointing ropp2bufr to the code- or user-defined preference is an excellent (and long-overdue) one.

(The documentation has already been updated to point out the need to link ropp2bufr_ecodes to ropp2bufr, if they are relying on a file of that name but want to use the ecCodes version of it.)

Consider for ROPP-11.0.

Change history (12)

comment:1 by olewis, 3 years ago

I have adjusted the configure.ac and Makefile.am in ropp_io to enable the building of individual executables for each of the bufr libraries.

On testing this however I have identified that the bufr2ropp_mo fails when bufr2ropp_ecmwf has also been built. This appears to be due to both the Metdb bufr library and the ECMWF bufr library having a function called BUFRREAD and so when the bufr2ropp_mo is ran it picks up the wrong routine from the ecmwf library.

comment:2 by Ian Culverwell, 3 years ago

Cc: olewis added

This is a bit odd. Before ecCodes we often had metDB and ECMWF BUFR together, and ropp2bufr and bufr2ropp both worked.

comment:3 by olewis, 3 years ago

I think this is because when we built with both MetDB and ECMWF BUFR it only built the MetDB version of both ropp2bufr and bufr2ropp and had this in the configre.ac which seems to stop the use of the ECMWF library.

# ecbufr and metdbbufr if test x$HAVE_LIBRARY_bufr = xyes ; then

if test x$HAVE_LIBRARY_metdbbufr = xyes ; then

AC_MSG_NOTICE([Both MetDB & ECMWF BUFR libraries found; using MetDB in preference]) AM_CONDITIONAL(HAVE_ECMWF_BUFR, test x$HAVE_LIBRARY_bufr = xno) AS_VAR_SET(HAVE_LIBRARY_bufr, no) LIBS="$NO_ECMWF_LIBS"

fi

fi

comment:4 by olewis, 3 years ago

The debug output from bufr2ropp_mo using the ropp test data is

---------------------------------------------------------------------
                  BUFR (MetDB) to ROPP Decoder
---------------------------------------------------------------------

INFO (from bufr2ropp):  Reading /data/users/olewis/ROPP_n688/ropp_io/data/ropp_test.bfr
forrtl: severe (174): SIGSEGV, segmentation fault occurred
Image              PC                Routine            Line        Source             
bufr2ropp_mo       00000000008EFC24  Unknown               Unknown  Unknown
libpthread-2.17.s  00002B5A95806630  Unknown               Unknown  Unknown
bufr2ropp_mo       00000000008D071E  Unknown               Unknown  Unknown
bufr2ropp_mo       0000000000404921  MAIN__                    281  bufr2ropp_mo.f90
bufr2ropp_mo       00000000004040FE  Unknown               Unknown  Unknown
libc-2.17.so       00002B5A95A35555  __libc_start_main     Unknown  Unknown
bufr2ropp_mo       000000000040400D  Unknown               Unknown  Unknown


comment:5 by olewis, 3 years ago

Line 281 of bufr2ropp_mo.f90 was calling a routine called bufrread. On searching for this it was found in both the mobufr library and the ecbufr library. I think when both ecbufr and mobufr were built it was calling the bufrread function from the bufr library and therefore failing.

The libraries were being built in an order which put the ecbufr library ahead of the mobufr library. The order that these are built can be changed by adjusting the configure.ac script. As libraries are added by placing the latest addition to the library list at the front of the list in the configure file then to change the order of the mobufr and ecbufr libraries the order of these being assigned in the configure.ac file needs to be reversed. This puts the ecbufr option before mobufr within the configure.ac file which then leads to it being added to the LIBS list before mobufr. This therefore means that the mobufr routines can access the correct BUFRREAD function when ran.

Testing the 8 configurations of mobufr eccodes and ecmwf bufr produces the following results

no eccodes  no metdb  no ecmwf    (Tick)
no eccodes  no metdb  ecmwf       (Tick)
no eccodes  metdb     ecmwf       (Tick eum2bufr fails because the test is using ropp2bufr(mo) to compare )
no eccodes  metdb     no ecmwf    (Tick)
eccodes     metdb     ecmwf       (Tick)  
eccodes     metdb     no ecmwf    (Tick)
eccodes     no metdb  no ecmwf    (Tick)
eccodes    no metdb   ecmwf       (Tick)

The only one to have a problem is the eum2bufr test with no eccodes and having both mobufr and ecmwf bufr. The test that fails is the eum2bufrb test which I think fails because it is comparing the bufr files of eum2bufr with eum2ropp followed by ropp2bufr. As the mobufr is the preferred option, the ropp2bufr will be set as ropp2bufr_mo. As the eum2bufr routine is associated with the ecmwf and eccodes libraries it means that the ropp2bufr routine used in this test is not the appropriate choice as it should be tested using ropp2bufr_ec in this case.

comment:6 by Ian Culverwell, 3 years ago

See the Suggested solutions in comment:ticket:699:10 for a connection to this work.

comment:7 by Ian Culverwell, 3 years ago

Milestone: 11.011.1

To be re-examined for ROPP-11.1.

comment:8 by Ian Culverwell, 3 years ago

Resolution: fixed
Status: newclosed

These changes have been introduced into the ropp_io/tools/Makefile.am and ropp_io/configure.ac files of the ROPP-11.1 make tests at r7042, r7044 and r7081. These check out OK: they build whatever BUFR execs they can, and link (e.g.) ropp2bufr to ropp2bufr_eccodes if that's available, else ropp2bufr_ecbufr if that's available, else ropp2bufr_mobufr if that's available, else ropp2bufr is undefined if none are available. The same is true of the $ROPP_ROOT/<compiler>/bin directory.

This requires, as Owen noted, building mobufr ahead of ecbufr. Owen's other point, about a failed test when (mobufr, ecbufr, eccodes) = (yes, yes, no), is answered by the root and branch restructuring of the core tests of ropp_io (see #273 and #699).

Closing ticket as work done.

comment:9 by Ian Culverwell, 3 years ago

Since we are now building different execs for different BUFR libs, it is a good idea to allow different BUFR Table version numbers for each exec. Do this by replacing

  INTEGER, PARAMETER :: VerMasTable = 12      ! Table version number (12)
! Users with access to a BUFR library that can handle it, such as eccodes-2.22.0,
! should replace the line above by this:
!  INTEGER, PARAMETER :: VerMasTable = 35      ! Table version number (35)

by

  INTEGER, PARAMETER :: VerMasTable_mo      = 12 ! Table version number (12 for MetDB versions </= 25.0.2)
  INTEGER, PARAMETER :: VerMasTable_ec      = 12 ! Table version number (12 for ECBUFR versions </= 000387)
  INTEGER, PARAMETER :: VerMasTable_eccodes = 35 ! Table version number (35 for ecCodes versions >/= 2.22.0)

in ropp_io/bufr/ropp2bufr_mod.f90, and using these 'tailored' table version numbers in the various ropp <--> bufr conversion routines.

It seems to work out: we now find

  22-22    Version of Master Table      12
  22-22    Version of Master Table      12

in the output of ropp2bufr_mo and ropp2bufr_ec, and

  22-22    Version of Master Table      35

in the output of ropp2bufr_eccodes.

Change committed at r7136.

comment:10 by Ian Culverwell, 3 years ago

Stig mentioned a problem building ecCodes with buildpack if BUFR_TABLES is not set. I cannot reproduce the problem, although adding

  if [[ -z $BUFR_TABLES ]]; then export BUFR_TABLES=$ROPP_ROOT/data/bufr/; fi
  mkdir -p $BUFR_TABLES > /dev/null 2>&1
  test_write BUFR_TABLES
  export INSTALL_DIR=$PREFIX

to the beginning of the build_ECCODES section of buildpack would appear to do the trick. This means build_ECCODES now mirrors build_ECBUFR.

I cannot reproduce Stig's problem. But we have seen instances when building ecCodes with buildpack causes failures like

Install the project...
-- Install configuration: "RelWithDebInfo"
-- Installing: /usr/local/include/eccodes_ecbuild_config.h
CMake Error at cmake_install.cmake:41 (file):
  file INSTALL cannot copy file
  "/data/users/idculv/ROPP/ropp_src/branches/dev/Share/ROPP110_prototype/eccodes-2.22.0-Source/tmpbuild/eccodes_ecbuild_config.h"
  to "/usr/local/include/eccodes_ecbuild_config.h": Permission denied.

Clearly this is a problem with the install directory, and the change above should fix this, regardless of the existence or not of $BUFR_TABLES. I don't think the change will cause any problems, so it has been committed at r7138.

comment:11 by Ian Culverwell, 3 years ago

The ropp_1dvar core test reference files have been updated to reflect the newish LevMarq default solver at r7139.

The lev1b data have been removed from the refrac reference files by using ropp2ropp -r 1b -m. There are no lev2a data in the bangle files, and trying to remove them with ropp2ropp -r 2a -m leads to the removal of the lev2e data. This sounds like a bug.

comment:12 by Ian Culverwell, 3 years ago

r7037:r7139 of https://trac.romsaf.org/ropp/browser/ropp_src/branches/dev/Share/ROPP111_make_test has been merged into https://trac.romsaf.org/ropp/browser/ropp_src/branches/dev/Share/ROPP111_prototype at r7140. Any further development will take place on the latter branch, so this ticket can probably be left alone now.

Note: See TracTickets for help on using tickets.