Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 64-bit rownrs in Dysco #1166

Merged
merged 2 commits into from
Mar 16, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 9 additions & 5 deletions tables/Dysco/dyscostman.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ casacore::Bool DyscoStMan::flush(casacore::AipsIO &,
return false;
}

void DyscoStMan::create(casacore::uInt nRow) {
void DyscoStMan::create64(casacore::rownr_t nRow) {
_nRow = nRow;
_fStream.reset(new std::fstream(
fileName().c_str(),
Expand Down Expand Up @@ -281,7 +281,7 @@ void DyscoStMan::initializeRowsPerBlock(size_t rowsPerBlock,
if (writeToHeader) writeHeader();
}

void DyscoStMan::open(casacore::uInt nRow, casacore::AipsIO &) {
casacore::rownr_t DyscoStMan::open64(casacore::rownr_t nRow, casacore::AipsIO &) {
_nRow = nRow;
_fStream.reset(new std::fstream(fileName().c_str(),
std::ios_base::in | std::ios_base::out));
Expand All @@ -302,6 +302,7 @@ void DyscoStMan::open(casacore::uInt nRow, casacore::AipsIO &) {
_nBlocksInFile = (size_t(size) - _headerSize) / _blockSize;
else
_nBlocksInFile = 0;
return nRow;
}

casacore::DataManagerColumn *DyscoStMan::makeScalarColumn(
Expand Down Expand Up @@ -344,7 +345,10 @@ casacore::DataManagerColumn *DyscoStMan::makeIndArrColumn(
"column desc constructor");
}

void DyscoStMan::resync(casacore::uInt /*nRow*/) {}
casacore::rownr_t DyscoStMan::resync64(casacore::rownr_t nRow)
{
return nRow;
}

void DyscoStMan::deleteManager() { unlink(fileName().c_str()); }

Expand Down Expand Up @@ -377,9 +381,9 @@ void DyscoStMan::prepare() {

void DyscoStMan::reopenRW() {}

void DyscoStMan::addRow(casacore::uInt nrrow) { _nRow += nrrow; }
void DyscoStMan::addRow64(casacore::rownr_t nrrow) { _nRow += nrrow; }

void DyscoStMan::removeRow(casacore::uInt rowNr) {
void DyscoStMan::removeRow64(casacore::rownr_t rowNr) {
if (rowNr != _nRow - 1)
throw DyscoStManError(
"Trying to remove a row in the middle of the file: "
Expand Down
10 changes: 5 additions & 5 deletions tables/Dysco/dyscostman.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,11 @@ class DyscoStMan : public casacore::DataManager {

// Let the storage manager create files as needed for a new table.
// This allows a column with an indirect array to create its file.
virtual void create(casacore::uInt nRow) final override;
virtual void create64(casacore::rownr_t nRow) final override;

// Open the storage manager file for an existing table.
// Return the number of rows in the data file.
virtual void open(casacore::uInt nRow, casacore::AipsIO &) final override;
virtual casacore::rownr_t open64(casacore::rownr_t nRow, casacore::AipsIO &) final override;

// Create a column in the storage manager on behalf of a table column.
// The caller will NOT delete the newly created object.
Expand All @@ -358,7 +358,7 @@ class DyscoStMan : public casacore::DataManager {
const casacore::String &name, int dataType,
const casacore::String &dataTypeID) final override;

virtual void resync(casacore::uInt nRow) final override;
virtual casacore::rownr_t resync64(casacore::rownr_t nRow) final override;

virtual void deleteManager() final override;

Expand All @@ -372,10 +372,10 @@ class DyscoStMan : public casacore::DataManager {
virtual void reopenRW() final override;

// Add rows to the storage manager.
virtual void addRow(casacore::uInt nrrow) final override;
virtual void addRow64(casacore::rownr_t nrrow) final override;

// Delete a row from all columns.
virtual void removeRow(casacore::uInt rowNr) final override;
virtual void removeRow64(casacore::rownr_t rowNr) final override;

// Do the final addition of a column.
virtual void addColumn(casacore::DataManagerColumn *) final override;
Expand Down
6 changes: 3 additions & 3 deletions tables/Dysco/dyscostmancol.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "dyscodistribution.h"
#include "dysconormalization.h"

#include <casacore/tables/DataMan/StManColumn.h>
#include <casacore/tables/DataMan/StManColumnBase.h>

#include <casa/Arrays/IPosition.h>

Expand All @@ -19,15 +19,15 @@ class DyscoStMan;
* Base class for columns of the DyscoStMan.
* @author André Dysco
*/
class DyscoStManColumn : public casacore::StManColumn {
class DyscoStManColumn : public casacore::StManColumnBase {
public:
/**
* Constructor, to be overloaded by subclass.
* @param parent The parent stman to which this column belongs.
* @param dtype The column's type as defined by Casacore.
*/
explicit DyscoStManColumn(DyscoStMan *parent, int dtype)
: casacore::StManColumn(dtype),
: casacore::StManColumnBase(dtype),
_offsetInBlock(0),
_storageManager(parent) {}

Expand Down
26 changes: 15 additions & 11 deletions tables/Dysco/threadeddyscocolumn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,20 @@ void ThreadedDyscoColumn<DataType>::loadBlock(size_t blockIndex) {

template <typename DataType>
void ThreadedDyscoColumn<DataType>::getValues(
casacore::uInt rowNr, casacore::Array<DataType> *dataPtr) {
casacore::rownr_t rowNr, casacore::Array<DataType> *dataArr) {
// Make sure array storage is contiguous.
casacore::Bool deleteIt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to start using just bool for new code?

DataType* dataPtr = dataArr->getStorage (deleteIt);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sometime in the future, it would be better to do this using a RAII structure. There are all kind of exceptions possible in the code below, which would lead to leaks. Not really directly a concern now, so no changes required, but maybe for a future MR.

if (!areOffsetsInitialized()) {
// Trying to read before first block was written -- return zero
// TODO if a few rows were written of the first block, those are
// incorrectly returned. This is a rare case but can be fixed.
for (typename casacore::Array<DataType>::contiter i = dataPtr->cbegin();
i != dataPtr->cend(); ++i)
*i = DataType();
*dataPtr = DataType();
} else {
size_t blockIndex = getBlockIndex(rowNr);
if (blockIndex >= nBlocksInFile()) {
// Trying to read a row that was not stored yet -- return zero
for (typename casacore::Array<DataType>::contiter i = dataPtr->cbegin();
i != dataPtr->cend(); ++i)
*i = DataType();
*dataPtr = DataType();
} else {
std::unique_lock<std::mutex> lock(_mutex);
// Wait until the block to be read is not in the write cache
Expand All @@ -137,9 +136,10 @@ void ThreadedDyscoColumn<DataType>::getValues(

// The time block encoder is now initialized and contains the unpacked
// block.
_timeBlockBuffer->GetData(getRowWithinBlock(rowNr), dataPtr->data());
_timeBlockBuffer->GetData(getRowWithinBlock(rowNr), dataPtr);
}
}
dataArr->putStorage (dataPtr, deleteIt);
}

template <typename DataType>
Expand Down Expand Up @@ -168,7 +168,10 @@ void ThreadedDyscoColumn<DataType>::storeBlock() {

template <typename DataType>
void ThreadedDyscoColumn<DataType>::putValues(
casacore::uInt rowNr, const casacore::Array<DataType> *dataPtr) {
casacore::rownr_t rowNr, const casacore::Array<DataType> *dataArr) {
// Make sure array storage is contiguous.
casacore::Bool deleteIt;
const DataType* dataPtr = dataArr->getStorage (deleteIt);
if (!areOffsetsInitialized()) {
// If the manager did not initialize its offsets yet, then it is determined
// from the first "time block" (a block with the same time, field and spw)
Expand Down Expand Up @@ -207,11 +210,12 @@ void ThreadedDyscoColumn<DataType>::putValues(
// Load new block
loadBlock(blockIndex);
}
_timeBlockBuffer->SetData(blockRow, ant1, ant2, dataPtr->data());
_timeBlockBuffer->SetData(blockRow, ant1, ant2, dataPtr);
} else {
_timeBlockBuffer->SetData(rowNr, ant1, ant2, dataPtr->data());
_timeBlockBuffer->SetData(rowNr, ant1, ant2, dataPtr);
}
_isCurrentBlockChanged = true;
dataArr->freeStorage (dataPtr, deleteIt);
}

template <typename DataType>
Expand Down
65 changes: 24 additions & 41 deletions tables/Dysco/threadeddyscocolumn.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace dyscostman {
class DyscoStMan;

/**
* A column for storing compressed values in a threaded way, tailered for the
* A column for storing compressed values in a threaded way, tailored for the
* data and weight columns that use a threaded approach for encoding.
* @author André Offringa
*/
Expand All @@ -42,7 +42,6 @@ class ThreadedDyscoColumn : public DyscoStManColumn {
ThreadedDyscoColumn(const ThreadedDyscoColumn &source) = delete;

void operator=(const ThreadedDyscoColumn &source) = delete;

/** Destructor. */
virtual ~ThreadedDyscoColumn();

Expand All @@ -51,7 +50,7 @@ class ThreadedDyscoColumn : public DyscoStManColumn {

/** Get the dimensions of the values in a particular row.
* The rownr parameter is not used as the shape is the same for all rows. */
virtual casacore::IPosition shape(casacore::uInt /*rownr*/) override {
virtual casacore::IPosition shape(casacore::rownr_t /*rownr*/) override {
return _shape;
}

Expand All @@ -61,18 +60,10 @@ class ThreadedDyscoColumn : public DyscoStManColumn {
* @param rowNr The row number to get the values for.
* @param dataPtr The array of values, which should be a contiguous array.
*/
virtual void getArrayComplexV(
casacore::uInt rowNr,
casacore::Array<casacore::Complex> *dataPtr) override {
// Note that this method is specialized for std::complex<float> -- the
// generic method won't do anything
return DyscoStManColumn::getArrayComplexV(rowNr, dataPtr);
}
virtual void getArrayfloatV(casacore::uInt rowNr,
casacore::Array<float> *dataPtr) override {
// Note that this method is specialized for float -- the generic method
// won't do anything
return DyscoStManColumn::getArrayfloatV(rowNr, dataPtr);
virtual void getArrayV(
casacore::rownr_t rowNr,
casacore::ArrayBase &dataPtr) override {
return DyscoStManColumn::getArrayV(rowNr, dataPtr);
}

/**
Expand All @@ -82,18 +73,10 @@ class ThreadedDyscoColumn : public DyscoStManColumn {
* @param rowNr The row number to write the values to.
* @param dataPtr The data pointer, which should be a contiguous array.
*/
virtual void putArrayComplexV(
casacore::uInt rowNr,
const casacore::Array<casacore::Complex> *dataPtr) override {
// Note that this method is specialized for std::complex<float> -- the
// generic method won't do anything
return DyscoStManColumn::putArrayComplexV(rowNr, dataPtr);
}
virtual void putArrayfloatV(casacore::uInt rowNr,
const casacore::Array<float> *dataPtr) override {
// Note that this method is specialized for float -- the generic method
// won't do anything
return DyscoStManColumn::putArrayfloatV(rowNr, dataPtr);
virtual void putArrayV(
casacore::rownr_t rowNr,
const casacore::ArrayBase &dataPtr) override {
return DyscoStManColumn::putArrayV(rowNr, dataPtr);
}

virtual void Prepare(DyscoDistribution distribution,
Expand Down Expand Up @@ -190,8 +173,8 @@ class ThreadedDyscoColumn : public DyscoStManColumn {

typedef std::map<size_t, CacheItem *> cache_t;

void getValues(casacore::uInt rowNr, casacore::Array<data_t> *dataPtr);
void putValues(casacore::uInt rowNr, const casacore::Array<data_t> *dataPtr);
void getValues(casacore::rownr_t rowNr, casacore::Array<data_t> *dataPtr);
void putValues(casacore::rownr_t rowNr, const casacore::Array<data_t> *dataPtr);

void stopThreads();
void encodeAndWrite(size_t blockIndex, const CacheItem &item,
Expand Down Expand Up @@ -228,24 +211,24 @@ class ThreadedDyscoColumn : public DyscoStManColumn {
};

template <>
inline void ThreadedDyscoColumn<std::complex<float>>::getArrayComplexV(
casacore::uInt rowNr, casacore::Array<casacore::Complex> *dataPtr) {
getValues(rowNr, dataPtr);
inline void ThreadedDyscoColumn<std::complex<float>>::getArrayV(
casacore::rownr_t rowNr, casacore::ArrayBase &dataPtr) {
getValues(rowNr, static_cast<casacore::Array<std::complex<float>>*>(&dataPtr));
}
template <>
inline void ThreadedDyscoColumn<std::complex<float>>::putArrayComplexV(
casacore::uInt rowNr, const casacore::Array<casacore::Complex> *dataPtr) {
putValues(rowNr, dataPtr);
inline void ThreadedDyscoColumn<std::complex<float>>::putArrayV(
casacore::rownr_t rowNr, const casacore::ArrayBase &dataPtr) {
putValues(rowNr, static_cast<const casacore::Array<std::complex<float>>*>(&dataPtr));
}
template <>
inline void ThreadedDyscoColumn<float>::getArrayfloatV(
casacore::uInt rowNr, casacore::Array<float> *dataPtr) {
getValues(rowNr, dataPtr);
inline void ThreadedDyscoColumn<float>::getArrayV(
casacore::rownr_t rowNr, casacore::ArrayBase &dataPtr) {
getValues(rowNr, static_cast<casacore::Array<float>*>(&dataPtr));
}
template <>
inline void ThreadedDyscoColumn<float>::putArrayfloatV(
casacore::uInt rowNr, const casacore::Array<float> *dataPtr) {
putValues(rowNr, dataPtr);
inline void ThreadedDyscoColumn<float>::putArrayV(
casacore::rownr_t rowNr, const casacore::ArrayBase &dataPtr) {
putValues(rowNr, static_cast<const casacore::Array<float>*>(&dataPtr));
}

extern template class ThreadedDyscoColumn<std::complex<float>>;
Expand Down