From 97473157f24e6e22aa77e45b4bfb0d9468a58721 Mon Sep 17 00:00:00 2001
From: Christian Zimmermann <chizeta@f3l.de>
Date: Tue, 18 Mar 2025 15:37:42 -0700
Subject: [PATCH] hdf5 + hdf5 mpi: clean up

---
 src/opt/hdf5/include/h5_dataset.h         |  9 +---
 src/opt/hdf5/lib/h5_dataset.cc            | 40 ++----------------
 src/opt/hdf5/tests/h5_basic_unit_test.cc  |  8 ++--
 src/opt/hdf5_mpi/include/h5_rdataset.cc.h |  7 +++-
 src/opt/hdf5_mpi/include/h5_rdataset.h    |  4 --
 src/opt/hdf5_mpi/lib/h5_rdataset.cc       | 51 +----------------------
 6 files changed, 14 insertions(+), 105 deletions(-)

diff --git a/src/opt/hdf5/include/h5_dataset.h b/src/opt/hdf5/include/h5_dataset.h
index 3c58cd3..ca0ad60 100644
--- a/src/opt/hdf5/include/h5_dataset.h
+++ b/src/opt/hdf5/include/h5_dataset.h
@@ -64,13 +64,6 @@ namespace CNORXZ
 	     */
 	    virtual Dataset& writebase(const RangePtr& dataRange, Sptr<YIndex> pos, const void* data);
 	    
-	    /** Read the dataset.
-		@param dest Pointer to destination.
-		@param readrange Range of the destination data.
-		@param beg Position within the file space.
-	     */
-	    virtual void readbase(void* dest, RangePtr readrange, Sptr<YIndex> beg) const;
-
 	    /** Read the dataset.
 		@param dest Pointer to destination.
 		@param tsize Size of dest data type.
@@ -91,7 +84,7 @@ namespace CNORXZ
 	    /** Get the data range.
 		@return Pointer to the range.
 	     */
-	    const RangePtr& dataRange() const;
+	    const RangePtr& fileRange() const;
 	    
 	protected:
 	    Vector<hsize_t> mkOff(const Sptr<YIndex>& beg) const;
diff --git a/src/opt/hdf5/lib/h5_dataset.cc b/src/opt/hdf5/lib/h5_dataset.cc
index 6e2a468..b1a5294 100644
--- a/src/opt/hdf5/lib/h5_dataset.cc
+++ b/src/opt/hdf5/lib/h5_dataset.cc
@@ -136,38 +136,6 @@ namespace CNORXZ
 	    H5Sclose(memspace);
 	    return *this;
 	}
-	
-	void Dataset::readbase(void* dest, RangePtr readRange, Sptr<YIndex> beg) const
-	{
-	    // TODO: Check if readRange is compatible with mFileRange!!!
-	    if(not readRange){
-		readRange = mFileRange;
-	    }
-	    else {
-		CXZ_ASSERT(readRange->dim() == mFileRange->dim(), "dimension of data range ("
-			   << readRange->dim() << ") different from dimension of file range ("
-			   << mFileRange->dim() << ")");
-	    }
-	    Vector<hsize_t> dims(readRange->dim());
-	    for(SizeT i = 0; i != dims.size(); ++i){
-		dims[i] = readRange->sub(i)->size();
-	    }
-	    if(beg){
-		CXZ_ASSERT(beg->range()->dim() == mFileRange->dim(), "dimension of position index ("
-			   << beg->range()->dim() << ") different from dimension of file range ("
-			   << mFileRange->dim() << ")");
-		const Vector<hsize_t> fpos = mkOff(beg);
-		H5Sselect_hyperslab(mFilespace, H5S_SELECT_SET, fpos.data(), NULL, dims.data(), NULL);
-	    }
-	    const hid_t mem_space_id = H5Screate_simple(static_cast<hsize_t>(dims.size()),
-							dims.data(), nullptr);
-	    const hid_t xfer_plist_id = this->mkdxpl();
-	    const herr_t err = H5Dread(mId, mType, mem_space_id, mFilespace, xfer_plist_id, dest);
-	    CXZ_ASSERT(err >= 0, "error while reading dataset '" << mName
-		       << "', errorcode :" << err);
-	    H5Pclose(xfer_plist_id);
-	    H5Sclose(mem_space_id);
-	}
 
 	void Dataset::readbase(void* dest, SizeT tsize, Sptr<YIndex> mbeg, Sptr<YIndex> mend,
 			       Sptr<YIndex> fbeg) const
@@ -175,12 +143,10 @@ namespace CNORXZ
 	    const SizeT D = mFileRange->dim();
 	    const bool todo = mbeg != nullptr;
 	    Vector<hsize_t> offset(D);
-	    Vector<hsize_t> moffset(D);//!!!
+	    Vector<hsize_t> moffset(D);
 	    Vector<hsize_t> dims(D);
-	    Vector<hsize_t> mdims(D);//!!!
+	    Vector<hsize_t> mdims(D);
 	    if(todo){
-		//RangePtr dr = mbeg->range();
-		//const bool parallel = true;
 		CXZ_ASSERT(mend != nullptr, "no end edge given");
 		CXZ_ASSERT(fbeg->range()->dim() == D, "dimension of file index ("
 			   << fbeg->range()->dim() << ") different from dimension of file range ("
@@ -229,7 +195,7 @@ namespace CNORXZ
 	    return xfer_plist_id;
 	}
 
-	const RangePtr& Dataset::dataRange() const
+	const RangePtr& Dataset::fileRange() const
 	{
 	    return mFileRange;
 	}
diff --git a/src/opt/hdf5/tests/h5_basic_unit_test.cc b/src/opt/hdf5/tests/h5_basic_unit_test.cc
index afad1eb..c8210c0 100644
--- a/src/opt/hdf5/tests/h5_basic_unit_test.cc
+++ b/src/opt/hdf5/tests/h5_basic_unit_test.cc
@@ -231,7 +231,7 @@ namespace
 	File h5f(mFileName, true);
 	h5f.open();
 	auto dset = h5f.getGroup("gr2")->open().getDataset("dat1", Double{});
-	YIndex beg(dset->dataRange());
+	YIndex beg(dset->fileRange());
 	beg.setSub(0,2);
 	YIndex end = beg - 1;
 	end.setSub(0,2);
@@ -252,12 +252,12 @@ namespace
 	File h5f(mFileName, true);
 	h5f.open();
 	auto dset = h5f.getGroup("gr2")->open().getDataset("dat1", Double{});
-	//YIndex beg(dset->dataRange());
+	//YIndex beg(dset->fileRange());
 	//beg.setSub(0,2);
 	//YIndex end = beg - 1;
 	//end.setSub(0,2);
 	//auto data = dset->read(beg,end);
-	Vector<RangePtr> dranges(dset->dataRange()->dim());
+	Vector<RangePtr> dranges(dset->fileRange()->dim());
 	EXPECT_EQ(dranges.size(), 5u);
 	dranges[0] = CRangeFactory(5).create();
 	dranges[1] = CRangeFactory(2).create();
@@ -268,7 +268,7 @@ namespace
 	MArray<Double> data(drange);
 	auto mbeg = data.begin();
 	auto mend = data.begin();
-	YIndex fbeg(dset->dataRange());
+	YIndex fbeg(dset->fileRange());
 	Arr<SizeT,5> mbegpos = { 1,0,0,2,0 };
 	Arr<SizeT,5> mendpos = { 3,1,3,4,1 };
 	Arr<SizeT,5> fbegpos = { 3,1,4,2,0 };
diff --git a/src/opt/hdf5_mpi/include/h5_rdataset.cc.h b/src/opt/hdf5_mpi/include/h5_rdataset.cc.h
index 265cb83..16bbc2e 100644
--- a/src/opt/hdf5_mpi/include/h5_rdataset.cc.h
+++ b/src/opt/hdf5_mpi/include/h5_rdataset.cc.h
@@ -40,8 +40,12 @@ namespace CNORXZ
 	mpi::RArray<T> SRDataset<T>::read(const RangePtr& geom) const
 	{
 	    auto rr = rangeCast<mpi::RRange<YRange,YRange>>( mpi::rrange(mFileRange, geom) );
+	    auto mbeg = std::make_shared<mpi::RIndex<YIndex,YIndex>>(rr);
+	    auto mend = std::make_shared<mpi::RIndex<YIndex,YIndex>>(rr);
+	    *mend = mend->lmax().val()-1;
+	    auto fbeg = std::make_shared<YIndex>(mFileRange);
 	    mpi::RArray<T> out(rr);
-	    readbase(out.data(), rr, nullptr);
+	    rreadbase(out.data(), sizeof(T), mbeg, mend, fbeg);
 	    return out;
 	}
 	
@@ -60,7 +64,6 @@ namespace CNORXZ
 	    auto mend = std::make_shared<mpi::RIndex<YIndex,YIndex>>(outrange);
 	    *mend = mend->lmax().val()-1;
 	    auto fbeg = std::make_shared<YIndex>(mFileRange);
-	    //readbase(out.data(), outrange, nullptr);
 	    rreadbase(out.data(), sizeof(T), mbeg, mend, fbeg);
 	    return out;
 	}
diff --git a/src/opt/hdf5_mpi/include/h5_rdataset.h b/src/opt/hdf5_mpi/include/h5_rdataset.h
index 30122f5..8b61397 100644
--- a/src/opt/hdf5_mpi/include/h5_rdataset.h
+++ b/src/opt/hdf5_mpi/include/h5_rdataset.h
@@ -36,14 +36,10 @@ namespace CNORXZ
 		@param _parent Parent content object.
 	     */
 	    RDataset(const String& name, const ContentBase* _parent);
-	    //virtual ~RDataset();
 
 	    virtual RDataset& initbase(const RangePtr& fileRange, hid_t type) override;
 	    virtual RDataset& writebase(const RangePtr& writeRange, Sptr<YIndex> pos,
 					const void* data) override;
-	    virtual void readbase(void* dest, RangePtr readrange, Sptr<YIndex> beg) const override;
-	    //virtual void readbase(void* dest, Sptr<YIndex> mbeg, Sptr<YIndex> mend,
-	    //			  Sptr<YIndex> fbeg) const override;
 
 	    virtual hid_t mkdxpl() const override;
 
diff --git a/src/opt/hdf5_mpi/lib/h5_rdataset.cc b/src/opt/hdf5_mpi/lib/h5_rdataset.cc
index 9c5ded1..00bcb15 100644
--- a/src/opt/hdf5_mpi/lib/h5_rdataset.cc
+++ b/src/opt/hdf5_mpi/lib/h5_rdataset.cc
@@ -153,56 +153,7 @@ namespace CNORXZ
 		Dataset::readbase(dest, tsize, mbegl, mendl, fbegl);
 	    }
 	}
-
-	void RDataset::readbase(void* dest, RangePtr readRange, Sptr<YIndex> beg) const
-	{
-	    RangePtr dr = readRange;
-	    if(not dr){
-		dr = mFileRange;
-	    }
-	    bool parallel = dr->stype() == "R";
-	    if(parallel){
-		dr = readRange->sub(1);
-	    }
-	    CXZ_ASSERT(dr->dim() == mFileRange->dim(), "dimension of data range ("
-		       << dr->dim() << ") different from dimension of file range ("
-		       << mFileRange->dim() << ")");
-	    Vector<hsize_t> offset(mFileRange->dim());
-	    if(parallel){
-		mpi::RIndex<YIndex,YIndex> idx(readRange);
-		idx.localize();
-		const SizeT rat = mpi::getNumRanks() / idx.rankI()->lmax().val();
-		assert(rat == 1); // for now...
-		assert(mpi::getRankNumber() == idx.rankI()->lex());
-		for(SizeT i = 0; i != offset.size(); ++i){
-		    offset[i] = idx.rankI()->pack().get(i)->lex() * dr->savesub(i)->size();
-		}
-	    }
-	    if(beg){
-		CXZ_ASSERT(beg->range()->dim() == mFileRange->dim(), "dimension of position index ("
-			   << beg->range()->dim() << ") different from dimension of file range ("
-			   << mFileRange->dim() << ")");
-		for(SizeT i = 0; i != offset.size(); ++i){
-		    offset[i] += beg->pack().get(i)->lex();
-		}
-	    }
-
-	    Vector<hsize_t> dims(mFileRange->dim());
-	    for(SizeT i = 0; i != dims.size(); ++i){
-		dims[i] = dr->sub(i)->size();
-	    }
-	    H5Sselect_hyperslab(mFilespace, H5S_SELECT_SET, offset.data(), NULL, dims.data(), NULL);
-	    const hid_t mem_space_id = H5Screate_simple(static_cast<hsize_t>(dims.size()),
-							dims.data(), nullptr);
-	    const hid_t xfer_plist_id = this->mkdxpl();
-	    const herr_t err = H5Dread(mId, mType, mem_space_id, mFilespace, xfer_plist_id, dest);
-	    CXZ_ASSERT(err >= 0, "error while reading dataset '" << mName
-		       << "', errorcode :" << err);
-	    H5Pclose(xfer_plist_id);
-	    H5Sclose(mem_space_id);
-	    MPI_Barrier(MPI_COMM_WORLD);
-	}
-
+	
 	hid_t RDataset::mkdxpl() const
 	{
 	    const hid_t xfer_plist_id = H5Pcreate(H5P_DATASET_XFER);