A test for mb_rt.c - ray tracing

[schwehr]

This is a place for me to take notes while looking at mb_rt.c. I don't totally understand the design of it and before I try to improve it, it needs to have good test coverage. It's a great candidate for fuzzing and microbenchmarks.

Lots of questions starting with:

• Why the global static model variable when a ptr is already passed? Can we just fix a few functions and drop the model variable poluting all of mb_rt's scope?
• What is MBBA?
• Is the assumption of plotting baked into this?

So far, I have a start to mb_rt.h and an alloc test:

#ifndef MBIO_MB_RT_H_
#define MBIO_MB_RT_H_

struct velocity_model {
    /* velocity model */
    int number_node;
    double *depth;
    double *velocity;
    int number_layer;
    int *layer_mode;
    double *layer_gradient;
    double *layer_depth_center;
    double *layer_depth_top;
    double *layer_depth_bottom;
    double *layer_vel_top;
    double *layer_vel_bottom;

    /* raytracing bookkeeping variables */
    int ray_status;
    int done;
    int outofbounds;
    int layer;
    int turned;
    int plot_mode;
    int number_plot_max;
    int number_plot;
    int sign_x;
    double xx;
    double zz;
    double xf;
    double zf;
    double tt;
    double dt;
    double tt_left;
    double vv_source;
    double pp;
    double xc;
    double zc;
    double radius;
    double *xx_plot;
    double *zz_plot;
};

#ifdef __cplusplus
extern "C" {
#endif

int mb_rt_init(int verbose, int number_node, double *depth, double *velocity, void **modelptr, int *error);
int mb_rt_deall(int verbose, void **modelptr, int *error);
int mb_rt(int verbose, void *modelptr, double source_depth, double source_angle, double end_time, int ssv_mode,
          double surface_vel, double null_angle, int nplot_max, int *nplot, double *xplot, double *zplot, double *x, double *z,
          double *travel_time, int *ray_stat, int *error);
int mb_rt_circular(int verbose, int *error);  // TODO(schwehr): This is why the global is needed, yes?
int mb_rt_quad1(int verbose, int *error);
int mb_rt_quad2(int verbose, int *error);
int mb_rt_quad3(int verbose, int *error);
int mb_rt_quad4(int verbose, int *error);
int mb_rt_get_depth(int verbose, double beta, int dir_sign, int turn_sign, double *depth, int *error);
int mb_rt_plot_circular(int verbose, int *error);
int mb_rt_line(int verbose, int *error);
int mb_rt_vertical(int verbose, int *error);

#ifdef __cplusplus
}  /* extern "C" */
#endif

#endif  /* MBIO_MB_RT_H_ */

and

#include "mb_rt.h"

#include <vector>

#include "gmock.h"
#include "gtest.h"
#include "mb_define.h"
#include "mb_status.h"

namespace {

TEST(MbRtTest, Alloc) {
  int verbose = 2;
  std::vector<double> dep{10.1, 20.2, 30.3};
  std::vector<double> vel{1518.1, 1519.2, 1518.3};
  struct velocity_model *model = nullptr;
  int error = -999;
  EXPECT_EQ(MB_SUCCESS, mb_rt_init(verbose, dep.size(), dep.data(), vel.data(),
                                   (void **)&model, &error));
  EXPECT_NE(nullptr, model);
  EXPECT_EQ(error, MB_ERROR_NO_ERROR);
  EXPECT_EQ(MB_SUCCESS, mb_rt_deall(verbose, (void **)&model, &error));
}

}  // namespace

Causing crashes is super easy to do. Give a too long size or having the two arrays be different lengths. What should size of 0 do?

[dwcaress]

Kurt,

On May 2, 2019, at 8:05 PM, Kurt Schwehr notifications@github.com wrote:

This is a place for me to take notes while looking at mb_rt.c. I don't totally understand the design of it and before I try to improve it, it needs to have good test coverage. It's a great candidate for fuzzing and microbenchmarks.

Lots of questions starting with:

• Why the global static model variable when a ptr is already passed? Can we just fix a few functions and drop the model variable poluting all of mb_rt's scope? I don’t know why I did this - using a static pointer variable seems bad - feel free to fix it.

• What is MBBA? Ancient history not worth explaining. We should change MBBA to MBIO in these statements.

• Is the assumption of plotting baked into this? Yes, it is used in mbvelocitytool.

Also, the notion of a revision id tied to the repository is meaningless with Git, so we should get rid of the svn_id and rcs_id strings and print statements throughout the codebase.

Cheers, Dave

---

David W. Caress Principal Engineer Seafloor Mapping Lab

Monterey Bay Aquarium Research Institute 7700 Sandholdt Road Moss Landing, CA 95039

caress@mbari.org http://www.mbari.org/~caress/

Phone: 831-775-1775

[schwehr]

Thanks for the quick response. I'll look at removing the static pointer after I get some tests in place.