adding Adios2 io support #113

seegyoung · 2024-09-09T17:48:55Z

support for serial adios2 file I/O (set Omega_h_USE_ADIOS2 ON)
MPI only with single process at the moment
testing with kokkos and coda not included

jacobmerson · 2024-09-10T03:33:31Z

/runtests

github-actions · 2024-09-10T03:45:34Z

Test Result: failure (details)

jacobmerson

Hi Seegyoung, from what I see so far it looks like things are on the right track.

However, it seems like there are some missing files in your commit. I don't see the Omega_h_adios2.hpp file, or where the write_adios2 code is located.

jacobmerson · 2024-09-10T03:35:32Z

cmake/FindADIOS2.cmake

We shouldn't need this since find_package should directly work since adios2 provides a config file.

jacobmerson · 2024-09-10T03:36:55Z

src/CMakeLists.txt

+  set(OLD_CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH})
+  set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/")


I think if you get rid of the config files these should not be needed.

jacobmerson · 2024-09-10T03:38:34Z

src/CMakeLists.txt

+  set(ADIOS_REQUIRED_VERSION 2.10)
+  if (Omega_h_USE_MPI)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DADIOS2_USE_MPI")
+    target_include_directories(omega_h PUBLIC "${ADIOS2_INCLUDE_DIR}")


don't need this if we use the adios2 cmake targets

jacobmerson · 2024-09-10T03:43:34Z

src/CMakeLists.txt

+  if (Omega_h_USE_MPI)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DADIOS2_USE_MPI")
+    target_include_directories(omega_h PUBLIC "${ADIOS2_INCLUDE_DIR}")
+    target_link_libraries(omega_h PUBLIC "${ADIOS2_LIBS}")


I think the target should be adios2::cxx11_mpi see: https://github.com/SCOREC/redev/blob/f4b1d75b6996f5ed0a7c87c25f9d146d22babc80/CMakeLists.txt#L54

cwsmith · 2024-09-10T13:44:05Z

src/osh2bp.cpp

+  Omega_h::Mesh mesh(&lib);
+  Omega_h::binary::read(argv[1], lib.world(), &mesh);
+  Omega_h::write_adios2(argv[2], &mesh);
+}


Suggested change

}

return 0;

}

cwsmith · 2024-09-10T13:47:56Z

Thank you @seegyoung. Looks good.

If it isn't there already, and while partitioned mesh support is under development, we should add a check to the adios writer and reader to ensure that the mesh is serial.

seegyoung · 2024-09-11T20:06:11Z

I will update based on all the input from both of you.

jacobmerson · 2024-09-16T21:44:11Z

src/adios2_io.cpp

+    for (int dim=0; dim <= mesh.dim(); dim++)
+      for (int tag=0; tag < mesh.ntags(dim); tag++) {
+        auto tagbase = mesh.get_tag(dim, tag);
+        if (tagbase->type() == OMEGA_H_I8)


You can use the apply_to_omega_h_types function which will avoid code duplication here. Function is defined here:

omega_h/src/Omega_h_defines.hpp

Line 86 in 00131eb

auto apply_to_omega_h_types(Omega_h_Type type, const F&& f) {

example use case:

omega_h/src/Omega_h_rcFields.cpp

Lines 335 to 339 in 00131eb

auto new_tag = apply_to_omega_h_types(

tag->type(), [&,this](auto t) -> std::unique_ptr<TagBase> {

using T = decltype(t);

return this->get_rc_mesh_tag_from_rc_tag(ent_dim, as<T>(tag));

});

jacobmerson · 2024-09-16T21:47:51Z

@seegyoung I still don't see the definitions for write_value and read_value for adios2.

seegyoung · 2024-09-17T04:34:40Z

Sorry about the trouble - missing files are added

cwsmith · 2024-09-17T13:24:59Z

@seegyoung Would you please remove the romulus-config.sh file? We maintain build instructions for various systems in the wiki: https://github.com/SCOREC/omega_h/wiki

seegyoung · 2024-09-17T13:33:04Z

OK I will do.

jacobmerson · 2024-12-03T18:33:50Z

Hi Seegyoung, did you have a chance to address the comments from Cameron and I?

seegyoung · 2024-12-06T07:57:23Z

Yes, it was removed on September 17th. Thanks for checking with me.

jacobmerson · 2024-12-12T20:19:56Z

@seegyoung can you update this pull request with your changes? I do not see the requested updates.

seegyoung · 2024-12-13T03:40:38Z

I am not good at GitHub so I don't know what to do to resolve the issue and what's the source of the issue. Can someone can do this for me please?

cwsmith · 2024-12-13T18:49:31Z

If you have changes that are committed into your local copy of the branch (i.e., on the SCOREC filesystem) you can run 'git push' to upload them to github. This PR will automatically be updated.

I'm available to discuss via WebEx if needed. Please send me an email and we can pick a time.

jacobmerson

@seegyoung i have added additional comments. Please address them and update this pull request.

jacobmerson · 2024-12-23T03:47:32Z

cmake/FindADIOS2.cmake

@@ -0,0 +1,104 @@
+# - Try to find ADIOS2


Please remove this file. It is not needed and will cause us issues in the future.

OK I will try to remove it although I don't know what the the alternative is.

adios2 creates a config file since it is built with CMake so, as long as you point omega_h to the correct path during the configure stage it should bring in everything it needs.

jacobmerson · 2024-12-23T03:51:03Z

src/Omega_h_adios2.hpp

+template <typename T>
+void write_value(adios2::IO &io, adios2::Engine &writer,
+        T val, std::string &name, bool global=false);
+
+template <typename T>
+void read_value(adios2::IO &io, adios2::Engine &reader,
+        T *val, std::string &name, bool global=false);
+
+void write_adios2(filesystem::path const& path, Mesh *mesh);
+Mesh read_adios2(filesystem::path const& path, Library* lib);


Where are the definitions of these functions? Also, should provide doxygen style comment describing what they do and what the inputs are. Particularly, what is the bool called global doing?

global is a flag to tell whether it reads/writes global array or not.

Ok, by global array do you mean global do you mean what adios2 calls global, or it's global indexes, or something else?

global flag indicates whether the value is global (the same for all processes) or not. If global, adios2 doesn't need an array to read/write a value of which index is equivalent to the process rank

jacobmerson · 2024-12-23T03:53:42Z

src/adios2_io.cpp

+  cout<<"\n--- Mesh loaded from \""<<inpath<<"\" ---\n";
+  print_info(&lib, mesh);
+
+//  Omega_h::Mesh mesh = build_box(world, OMEGA_H_SIMPLEX, 1., 1., 0., 2, 2, 0);


Remove commented code.

jacobmerson · 2024-12-23T03:55:49Z

src/adios2_io.cpp

+ *
+ *  Created on: Aug 25, 2024
+ *  Author: Seegyoung Seol [email protected]
+ *


Remove this. It is already encoded in the git history.

jacobmerson · 2024-12-23T03:56:09Z

src/adios2_io.cpp

+
+// printTagInfo and getNumEq copied from describe.cpp
+template <typename T>
+void printTagInfo(Omega_h::Mesh mesh, std::ostringstream& oss, int dim, int tag, std::string type) {


jacobmerson · 2024-12-23T03:56:19Z

src/adios2_io.cpp

+}
+
+template <typename T>
+int getNumEq(Omega_h::Mesh mesh, std::string tagname, int dim, int value) {


jacobmerson · 2024-12-23T03:57:33Z

src/adios2_io.cpp

+using namespace Omega_h;
+using namespace std;
+
+// printTagInfo and getNumEq copied from describe.cpp


Why are you copying code here from describe? If that functionality is needed in multiple,e places refactor to a function and call function both places.

jacobmerson · 2024-12-23T04:00:10Z

src/bp2osh.cpp

+    exit(EXIT_FAILURE);
+  }
+  OMEGA_H_CHECK(argc == 3);
+  Omega_h::Mesh mesh = read_adios2(argv[1], &lib);


This naming is not consistent with how other writer/reader are named in omegah typically they have a namespace for each file type.

jacobmerson · 2024-12-23T04:01:33Z

src/Omega_h_adios2.hpp

+void read_value(adios2::IO &io, adios2::Engine &reader,
+        T *val, std::string &name, bool global=false);
+
+void write_adios2(filesystem::path const& path, Mesh *mesh);


To be consistent this should be inside adios2 namespace and just be called "write"

adios2 has namespace "adios2"

jacobmerson · 2024-12-31T06:07:32Z

src/Omega_h_adios2.cpp

@@ -462,7 +462,7 @@ void read_parents(adios2::IO &io, adios2::Engine &reader, Mesh* mesh, std::strin
  }
 }

-void write_adios2(adios2::IO &io, adios2::Engine & writer,
+void _write_adios2(adios2::IO &io, adios2::Engine & writer,


Why is this named _write_adios2? We generally avoid having names that start with underscore or double underscore in C++ because depending on the second symbol it could be undefined behavior.

see: https://en.cppreference.com/w/cpp/language/identifiers

jacobmerson · 2024-12-31T06:10:42Z

src/Omega_h_adios2.hpp

@@ -18,6 +18,8 @@ template <typename T>
 void read_value(adios2::IO &, adios2::Engine &reader,
        T *val, std::string &name, bool global=false);

+void write_adios2(filesystem::path const& path,
+                  std::map<Mesh*, std::string>& mesh_map);


Why is this stored as a map? Instead of a list of pairs or struct that holds a name and a omega_h mesh pointer?

In this case, a map is better as it takes less coding and easy to use. I will change it to a list of pairs only if you insist.

jacobmerson · 2024-12-31T06:15:42Z

src/Omega_h_adios2.cpp

+long unsigned int comm_size, rank;
+
+template <typename T>
+void write_value(adios2::IO &io, adios2::Engine &writer, 


make static

jacobmerson · 2024-12-31T06:15:56Z

src/Omega_h_adios2.cpp

+
+template <typename T>
+void read_value(adios2::IO &io, adios2::Engine &reader, 
+	T *val, std::string &name, bool global)


make static

jacobmerson · 2024-12-31T06:16:06Z

src/Omega_h_adios2.cpp

+template <typename T>
+void write_array(adios2::IO &io, adios2::Engine &writer, Read<T> array, 
+		std::string &name)
+{


make static

jacobmerson · 2024-12-31T06:24:32Z

src/Omega_h_adios2.cpp

+  std::string name = pref+"ntags_" + to_string(d);
+  write_value(io, writer, mesh->ntags(d), name, true);
+
+  for (int32_t i = 0; i < mesh->ntags(d); ++i)


should use Int since that's the size of ntags

This is what's in Omega_h_defines.hpp
typedef std::int32_t I32;
typedef I32 Int;
typedef I32 LO;
typedef I32 ClassId;

jacobmerson · 2024-12-31T06:26:40Z

src/Omega_h_adios2.cpp

+}
+
+void write_pbdry(adios2::IO &io, adios2::Engine &writer, Mesh* mesh, int d, std::string pref)
+{


rename to write_partition_boundary, in general prefer longer, but more informative names.

make static

jacobmerson · 2024-12-31T06:27:07Z

src/Omega_h_adios2.cpp

+  write_array(io, writer, owners.idxs, name);
+}
+
+void read_pbdry(adios2::IO &io, adios2::Engine &reader, Mesh* mesh, int d, std::string pref)


rename to read_partition_boundary

make static

jacobmerson · 2024-12-31T06:31:11Z

src/Omega_h_adios2.cpp

+
+
+void write_adios2(filesystem::path const& path, Mesh* mesh, std::string pref)
+{


I think you have your api backwards which is causing some issues. I don't think _write_adios2 should exist, but instead, this function should write to the specified path. If you want to have the function signature that takes a map it will just loop through and call this function multiple times like you have done with write_adios2.

jacobmerson · 2024-12-31T06:32:16Z

src/Omega_h_adios2.hpp

+template <typename T>
+void write_value(adios2::IO &, adios2::Engine &writer,
+        T val, std::string &name, bool global=false);
+
+template <typename T>
+void read_value(adios2::IO &, adios2::Engine &reader,
+        T *val, std::string &name, bool global=false);
+


These functions should not be exposed to the user since they are internal functions. You do not need to declare these in the header file at all since you only use in a single cpp file.

seegyoung added 4 commits August 25, 2024 02:19

support for Omega_h_USE_ADIOS2 added

c786f61

checking in adios2 dev

788e45c

adding osh2bp and bp2osh to utils

906c8bc

minor changes

dcdfe09

seegyoung requested review from cwsmith and jacobmerson September 9, 2024 17:49

seegyoung added the enhancement New feature or request label Sep 9, 2024

jacobmerson requested changes Sep 10, 2024

View reviewed changes

cwsmith reviewed Sep 10, 2024

View reviewed changes

cwsmith added the adios2 label Sep 10, 2024

adding missing header file and sanity check

a3fe049

jacobmerson reviewed Sep 16, 2024

View reviewed changes

adding adios2 impl (cpp) and config.sh for romulus

ad95f00

config.sh removed

a76e37a

resolve conflicts

cb35d7f

jacobmerson reviewed Dec 23, 2024

View reviewed changes

adding prefix option in adios2 read/write

5627b34

seegyoung added 2 commits December 29, 2024 21:28

writing multiple meshes with prefix

1971fe0

writing multiple meshes added to adios2_io test

aae867b

jacobmerson requested changes Dec 31, 2024

View reviewed changes

seegyoung added 2 commits January 15, 2025 23:47

fixing ctest error with adios2_io

f0099e7

removing unnecessary function declaration

6285327

		set(OLD_CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH})
		set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/")

	auto new_tag = apply_to_omega_h_types(
	tag->type(), [&,this](auto t) -> std::unique_ptr<TagBase> {
	using T = decltype(t);
	return this->get_rc_mesh_tag_from_rc_tag(ent_dim, as<T>(tag));
	});



		void write_adios2(filesystem::path const& path, Mesh* mesh, std::string pref)
		{

adding Adios2 io support #113

Are you sure you want to change the base?

adding Adios2 io support #113

Conversation

seegyoung commented Sep 9, 2024

jacobmerson commented Sep 10, 2024

github-actions bot commented Sep 10, 2024

jacobmerson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwsmith commented Sep 10, 2024

seegyoung commented Sep 11, 2024

Choose a reason for hiding this comment

jacobmerson commented Sep 16, 2024

seegyoung commented Sep 17, 2024

cwsmith commented Sep 17, 2024

seegyoung commented Sep 17, 2024

jacobmerson commented Dec 3, 2024

seegyoung commented Dec 6, 2024

jacobmerson commented Dec 12, 2024

seegyoung commented Dec 13, 2024

cwsmith commented Dec 13, 2024

jacobmerson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment