Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ endif()
########################################################################################################################
### dependencies and options

ecbuild_find_package( NAME eckit VERSION 1.4 REQUIRED )
ecbuild_find_package( NAME eckit VERSION 1.29 REQUIRED )

ecbuild_add_option( FEATURE FORTRAN
DESCRIPTION "whether or not to build the Fortran interface"
Expand Down
7 changes: 5 additions & 2 deletions src/odc/Indexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
* granted to it by virtue of its status as an intergovernmental organisation nor
* does it submit to any jurisdiction.
*/

#include "odc/Indexer.h"

#include "eckit/eckit.h"
#include <cstddef>
#include <utility>
#include <vector>

#include "eckit/io/DataHandle.h"
#include "eckit/io/Length.h"
#include "eckit/io/Offset.h"
#include "eckit/io/PartFileHandle.h"
Expand Down
6 changes: 4 additions & 2 deletions src/odc/ODBAPISettings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
#include "eckit/io/FileHandle.h"
#include "eckit/log/Log.h"
#include "eckit/thread/ThreadSingleton.h"
#include "eckit/utils/Literals.h"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely requires a bump to the minimum version of eckit, in the CMakeLists.txt.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes good catch!

#include "eckit/utils/StringTools.h"

#include "odc/LibOdc.h"
#include "odc/ODBAPISettings.h"

using namespace eckit;
using namespace eckit::literals;
using namespace std;

template class eckit::ThreadSingleton<odc::ODBAPISettings>;
Expand All @@ -47,7 +49,7 @@ void odc::ODBAPISettings::setHome(const char* argv0) {
::free(absoluteArgv0);
}
else if (argv0[0] == '.' && argv0[1] == '/') {
size_t bufferLen = 1024 * 8;
constexpr size_t bufferLen = 8_KiB;
char buffer[bufferLen];
full = string(::getcwd(buffer, bufferLen)) + string(argv0 + 1);
}
Expand All @@ -58,7 +60,7 @@ void odc::ODBAPISettings::setHome(const char* argv0) {
if (PathName(ps[i] + "/" + argv0).exists()) {
full = ps[i] + "/" + argv0;
if (ps[i][0] != '/') {
size_t bufferLen = 1024 * 8;
constexpr size_t bufferLen = 8_KiB;
char buffer[bufferLen];
full = string(::getcwd(buffer, bufferLen)) + full;
}
Expand Down
4 changes: 1 addition & 3 deletions src/odc/RowsCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@

#pragma once

namespace eckit {
class PathName;
}
#include <eckit/filesystem/PathName.h>

namespace odc {

Expand Down
2 changes: 1 addition & 1 deletion src/odc/WriterBufferingIterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ double* WriterBufferingIterator::data() {
}
double& WriterBufferingIterator::data(size_t i) {
ASSERT(initialisedColumns_);
ASSERT(i >= 0 && i < columns().size());
ASSERT(i < columns().size());
return nextRow_[columnOffsets_[i]];
}

Expand Down
36 changes: 17 additions & 19 deletions src/odc/WriterDispatchingIterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,10 @@

using namespace odc::core;


namespace {
// n.b. Duplicated from eckit::sql::expression::function::FunctionEQ::trimStringInDouble.
// TODO: Put somewhere better.
void trimStringInDouble(char*& p, size_t& len) {
len = 0;
for (; len < sizeof(double) && isprint(p[len]); ++len)
;
for (; len > 0 && isspace(p[len - 1]); --len)
;
size_t plen = len;
for (char* pp = p; isspace(*p) && p < pp + plen;) {
++p;
--len;
}
}
} // namespace

namespace odc {

//----------------------------------------------------------------------------------------------------------------------


template <typename WRITE_ITERATOR, typename OWNER>
WriterDispatchingIterator<WRITE_ITERATOR, OWNER>::WriterDispatchingIterator(OWNER& owner, int maxOpenFiles,
bool append) :
Expand Down Expand Up @@ -103,6 +84,23 @@ int WriterDispatchingIterator<WRITE_ITERATOR, OWNER>::setBitfieldColumn(size_t i
template <typename WRITE_ITERATOR, typename OWNER>
std::string WriterDispatchingIterator<WRITE_ITERATOR, OWNER>::generateFileName(const double* values,
unsigned long count) {
// n.b. Duplicated from eckit::sql::expression::function::FunctionEQ::trimStringInDouble.
// Provided as static lambda because this .cc file is included in the header and even when the
// function is declared in an annonymos workspace the declaration is marked as unneeded and emits
// a warning.
// TODO: Put somewhere better.
static const auto trimStringInDouble = [](char*& p, size_t& len) {
len = 0;
for (; len < sizeof(double) && isprint(p[len]); ++len)
;
for (; len > 0 && isspace(p[len - 1]); --len)
;
size_t plen = len;
for (char* pp = p; isspace(*p) && p < pp + plen;) {
++p;
--len;
}
};
std::string fileName(outputFileTemplate_);
int diff(0);
for (TemplateParameters::iterator it(templateParameters_.begin()); it != templateParameters_.end(); ++it) {
Expand Down
8 changes: 4 additions & 4 deletions src/odc/api/ColumnInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ struct ColumnInfo {
};

/** Column name */
std::string name;
std::string name{};
/** Column data type */
ColumnType type;
ColumnType type{};
/** Size of a single decoded value in bytes */
size_t decodedSize;
size_t decodedSize{};
/** List of bit and bit groups associated with a bitfield column */
std::vector<Bit> bitfield;
std::vector<Bit> bitfield{};
};

//----------------------------------------------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/odc/api/odc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ int odc_close(const odc_reader_t* reader) {
int odc_new_frame(odc_frame_t** frame, odc_reader_t* reader) {
return wrapApiFunction([frame, reader] {
ASSERT(reader);
(*frame) = new odc_frame_t{*reader, false};
(*frame) = new odc_frame_t{*reader, false, {}, {}};
});
}

Expand Down Expand Up @@ -835,7 +835,7 @@ int odc_encoder_set_data_array(odc_encoder_t* encoder, const void* data, long wi
int odc_encoder_add_column(odc_encoder_t* encoder, const char* name, int type) {
return wrapApiFunction([encoder, name, type] {
ASSERT(encoder);
encoder->columnInfo.emplace_back(ColumnInfo{std::string(name), ColumnType(type)});
encoder->columnInfo.emplace_back(ColumnInfo{std::string(name), ColumnType(type), {}, {}});
encoder->columnData.emplace_back(odc_encoder_t::EncodeColumn{0, 0});
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/odc/codec/CodecOptimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ int CodecOptimizer::setOptimalCodecs(core::MetaData& columns) {
case api::STRING: {
n = col.coder().numStrings();
ASSERT(n < 65536);

if (n == 1 && col.coder().dataSizeDoubles() == 1)
codec = "constant_string";
else if (n == 1 && std::getenv("ODC_ENABLE_WRITING_LONG_STRING_CODEC") != NULL)
Expand All @@ -106,7 +107,6 @@ int CodecOptimizer::setOptimalCodecs(core::MetaData& columns) {
codec = "int8_string";
else if (n < 65536)
codec = "int16_string";

std::unique_ptr<core::Codec> newCodec =
core::CodecFactory::instance().build<ByteOrder>(codec, col.type());
if (codec == "constant_string") {
Expand Down
4 changes: 3 additions & 1 deletion src/odc/core/Table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,9 @@ Span Table::decodeSpan(const std::vector<std::string>& columns) {
// Do the decoding

std::vector<size_t> lastDecoded(ncols, 0);
double decodeBuffer[maxDoublesDecode];

std::vector<double> decodeBufferVec(maxDoublesDecode);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a change we want to make. This shifts having the decode buffer from being on the stack to in the heap.

auto decodeBuffer = decodeBufferVec.data();

for (size_t rowCount = 0; rowCount < nrows; ++rowCount) {

Expand Down
6 changes: 4 additions & 2 deletions src/odc/csv/TextReaderIterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,10 @@ bool TextReaderIterator::next() {
std::vector<std::string> values(S::split(delimiter_, line));

size_t nCols = values.size();
if (nCols == 0)
return !(noMore_ = true);
if (nCols == 0) {
noMore_ = true;
return false;
}
ASSERT(nCols == columns().size());

for (size_t i = 0; i < nCols; ++i) {
Expand Down
4 changes: 0 additions & 4 deletions src/odc/tools/ODAHeaderTool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ class OffsetsPrinter : public MDPrinter {
Length length(tbl.nextPosition() - tbl.startPosition());
o << offset << " " << length << " " << tbl.rowCount() << " " << tbl.columnCount() << std::endl;
}

private:

unsigned long headerCount_;
};

class DDLPrinter : public MDPrinter {
Expand Down
2 changes: 1 addition & 1 deletion src/odc/tools/SplitTool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ using namespace std;
namespace odc {
namespace tool {

SplitTool::SplitTool(int argc, char* argv[]) : Tool(argc, argv), sort_(false), maxOpenFiles_(200) {
SplitTool::SplitTool(int argc, char* argv[]) : Tool(argc, argv), maxOpenFiles_(200), sort_(false) {
registerOptionWithArgument("-maxopenfiles");
}

Expand Down
4 changes: 2 additions & 2 deletions src/odc/tools/TestInt16_MissingCodec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ class MockReaderIterator3 {
data_ = columns_[0]->coder().missingValue();
break;
default:
return !(noMore_ = true);
break;
noMore_ = true;
return false;
}
return true;
}
Expand Down
2 changes: 0 additions & 2 deletions src/odc/tools/TestIntegerValues.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ static void test() {
}

std::cout << std::endl;
int nrows = 0;
for (; it != odb.end(); ++it) {
++nrows;
for (size_t i = 0; i < it->columns().size(); ++i) {
// float nr = ((*it)[i]); /// <- WRONG!
double nr = ((*it)[i]);
Expand Down
12 changes: 6 additions & 6 deletions tests/api/odc_encode_custom.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,24 @@ void create_scratch_data(size_t nrows, char data0[][8], int64_t data1[], char da
long integer_pool[] = {1234, 4321, Settings::integerMissingValue()};
int integer_pool_size = sizeof(integer_pool) / sizeof(integer_pool[0]);

long missing_integers[nrows];
cycle_longs(missing_integers, nrows, integer_pool, integer_pool_size);
std::vector<long> missing_integers(nrows);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular objection to this, but is there a reason to make this change?

cycle_longs(missing_integers.data(), nrows, integer_pool, integer_pool_size);

// Prepare the list of double values, including the missing value

double double_pool[] = {12.34, 43.21, Settings::doubleMissingValue()};
int double_pool_size = sizeof(double_pool) / sizeof(double_pool[0]);

double missing_doubles[nrows];
cycle_doubles(missing_doubles, nrows, double_pool, double_pool_size);
std::vector<double> missing_doubles(nrows);
cycle_doubles(missing_doubles.data(), nrows, double_pool, double_pool_size);

// Prepare the list of bitfield values

long bitfield_pool[] = {Ob00000001, Ob00001011, Ob01101011};
int bitfield_pool_size = sizeof(bitfield_pool) / sizeof(bitfield_pool[0]);

long bitfield_values[nrows];
cycle_longs(bitfield_values, nrows, bitfield_pool, bitfield_pool_size);
std::vector<long> bitfield_values(nrows);
cycle_longs(bitfield_values.data(), nrows, bitfield_pool, bitfield_pool_size);

// Fill in the passed data arrays with scratch values
for (size_t i = 0; i < nrows; i++) {
Expand Down
2 changes: 1 addition & 1 deletion tests/api/read.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ CASE("Where Span interface is used to read values without decoding") {
}

// Check string values
for (const std::string val : expver_vals) {
for (const auto& val : expver_vals) {
EXPECT(val == "xxxx");
}

Expand Down
2 changes: 0 additions & 2 deletions tests/c_api/encode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ CASE("Encode data with custom stride") {
const int ncols = 5;

// Construct some source data

long icol[2 * nrows] = {1111, 2222, 3333, 4444, 5555, 6666, 7777, 8888, 9999, 0};
long bcol[2 * nrows] = {1101, 2202, 3303, 4404, 5505, 6606, 7707, 8808, 9909, 0};
char scol[2 * nrows][3 * sizeof(double)] = {0};
Expand Down Expand Up @@ -421,7 +420,6 @@ CASE("Encode with more rows that fit inside a table") {
const int ncols = 5;

// Construct some source data

long icol[nrows] = {1111, 2222, 3333, 4444, 5555, 6666, 7777, 8888, 9999, 0};
long bcol[nrows] = {1101, 2202, 3303, 4404, 5505, 6606, 7707, 8808, 9909, 0};
char scol[nrows][3 * sizeof(double)] = {0};
Expand Down
Loading