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

Feature/sina/hdf5 output support #1480

Open
wants to merge 46 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
3f09c5e
created a clean new branch from Gabriel's work to push to github repo
doutriaux1 Dec 19, 2024
e0cd5bf
Comments Implemented
gwaegner Dec 19, 2024
d2f38ac
More Comments
gwaegner Dec 19, 2024
6c32398
Fix?
gwaegner Dec 19, 2024
31242a6
Fix??
gwaegner Dec 19, 2024
aa9073d
Final Fix
gwaegner Dec 19, 2024
b553c44
Axom Changes
gwaegner Dec 20, 2024
c5c0cb4
Brian's Changes, Wait to hear back on createFromNode
gwaegner Jan 8, 2025
8dd8a84
Merge branch 'develop' into sina_hdf5_implementation
doutriaux1 Jan 9, 2025
8e6afc1
Create From Node Changes
gwaegner Jan 24, 2025
247591f
Forgot to delete some leftover comments
gwaegner Jan 24, 2025
703dbd1
Small fix
gwaegner Jan 24, 2025
e1d7631
Merge remote-tracking branch 'llnl/develop' into sina_hdf5_implementa…
doutriaux1 Jan 25, 2025
0df65b3
merged github's develop in
doutriaux1 Jan 25, 2025
e118d24
Pipeline Fix
gwaegner Jan 27, 2025
0f5851e
Clang Format
gwaegner Jan 28, 2025
1168000
More ClangFormat
gwaegner Jan 28, 2025
429d9ce
bringing sumbodule back to develop version
doutriaux1 Feb 3, 2025
b71b05d
New Changes + Guarding
gwaegner Feb 4, 2025
0b689db
Merge remote-tracking branch 'llnl/feature/sina/hdf5_output_support' …
doutriaux1 Feb 4, 2025
ccfa513
bringing in submodule updates
doutriaux1 Feb 4, 2025
c05979d
Build Fix
gwaegner Feb 4, 2025
7c5d7cc
Documentation Update
gwaegner Feb 6, 2025
d036b67
Merge branch 'develop' into feature/sina/hdf5_output_support
doutriaux1 Feb 10, 2025
79f3729
Fixes submodule pointers
kennyweiss Feb 18, 2025
f92080d
Merge branch 'develop' into feature/sina/hdf5_output_support
kennyweiss Feb 18, 2025
4310ee1
Fixes includes in sina
kennyweiss Feb 19, 2025
0fd3a35
Guards usage of sina::Protocol::HDF5
kennyweiss Feb 19, 2025
1145c29
Guarding and Fortran tests
gwaegner Feb 22, 2025
4043d3e
Merge branch 'develop' into feature/sina/hdf5_output_support
doutriaux1 Feb 24, 2025
3ccfbb3
submodule fix?
doutriaux1 Feb 24, 2025
9429b60
more submodule fix
doutriaux1 Feb 24, 2025
45ee75b
Fix Fortran Test
gwaegner Feb 24, 2025
0f016bb
CMakeLists Fix
gwaegner Feb 24, 2025
4a2cf7e
Minor Fix
gwaegner Feb 24, 2025
2499896
Another minor change
gwaegner Feb 25, 2025
209ee7d
Maybe this change?
gwaegner Feb 25, 2025
35bf45b
Syntax fixes
gwaegner Feb 26, 2025
64e5026
Test Update
gwaegner Feb 26, 2025
a0341da
Syntax Fix
gwaegner Feb 26, 2025
baf2492
Test Pipeline for Not HDF5
gwaegner Feb 27, 2025
3e03355
Reset
gwaegner Feb 28, 2025
8370a21
New Changes
gwaegner Mar 7, 2025
50f17e1
Minor Fix
gwaegner Mar 11, 2025
6160d87
Merge branch 'develop' into feature/sina/hdf5_output_support
gwaegner Mar 11, 2025
fb3554d
Merge branch 'develop' into feature/sina/hdf5_output_support
gwaegner Mar 11, 2025
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
11 changes: 9 additions & 2 deletions src/axom/sina/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,17 @@ blt_list_append( TO sina_headers ELEMENTS core/AdiakWriter.hpp IF AXOM_USE_ADIAK
blt_list_append( TO sina_sources ELEMENTS core/AdiakWriter.cpp IF AXOM_USE_ADIAK )

# Add fortran interface for Sina
if(AXOM_USE_HDF5 AND ENABLE_FORTRAN)
set(AXOM_USE_HDF5_FORTRAN ".true.")
else()
set(AXOM_USE_HDF5_FORTRAN ".false.")
endif()
Comment on lines +56 to +60
Copy link
Member

Choose a reason for hiding this comment

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

As an aside, exposing the enum values will be a lot cleaner w/ shroud

Copy link
Author

Choose a reason for hiding this comment

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

@kennyweiss yes at some point we should convert to shroud as a separate PR. But it's kind of "if it's ain't broken why fix it?" at the moment 😉


if (ENABLE_FORTRAN)
blt_list_append( TO sina_headers ELEMENTS interface/sina_fortran_interface.h)
blt_list_append( TO sina_sources
ELEMENTS interface/sina_fortran_interface.cpp interface/sina_fortran_interface.f)
ELEMENTS interface/sina_fortran_interface.cpp interface/sina_fortran_interface.f.in)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to include the template file interface/sina_fortran_interface.f.in,

Copy link

Choose a reason for hiding this comment

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

Updated to remove .f.in's inclusion

configure_file(interface/sina_fortran_interface.f.in interface/sina_fortran_interface.f @ONLY)
Copy link
Member

Choose a reason for hiding this comment

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

We'll likely also need to install the generated file interface/sina_fortran_interface.f

Copy link

Choose a reason for hiding this comment

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

.f file is now explicitly installed.

endif()

#------------------------------------------------------------------------------
Expand Down Expand Up @@ -87,4 +94,4 @@ endif()

if(AXOM_ENABLE_EXAMPLES)
add_subdirectory(examples)
endif()
endif()
30 changes: 25 additions & 5 deletions src/axom/sina/core/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
*
******************************************************************************
*/

#include "axom/sina/core/Document.hpp"

#include "axom/config.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

Note: We need to #include "axom/config.hpp" to bring in compiler defines such as AXOM_USE_HDF5. If we don't, they will always appear to be off.

I updated the includes in this branch to bring in axom/config.hpp

Expand Down Expand Up @@ -66,6 +65,21 @@ void protocolWarn(std::string const protocol, std::string const &name)
}
}

std::string get_supported_file_types()
{
std::string types = "[";
for(size_t i = 0; i < supported_types.size(); ++i)
{
types += supported_types[i];
if(i < supported_types.size() - 1)
{
types += ", ";
}
}
types += "]";
return types;
}

void Document::add(std::unique_ptr<Record> record)
{
records.emplace_back(std::move(record));
Expand Down Expand Up @@ -313,8 +327,11 @@ void saveDocument(Document const &document,
#endif
else
{
throw std::invalid_argument(
"Invalid format choice. Please enter 'json' or 'hdf5'.");
std::ostringstream message;
message << "Invalid format choice. Please choose from one of the supported "
"protocols: "
<< get_supported_file_types();
throw std::invalid_argument(message.str());
}

if(rename(tmpFileName.c_str(), fileName.c_str()) != 0)
Expand Down Expand Up @@ -347,16 +364,19 @@ Document loadDocument(std::string const &path,
file_in.close();
node.parse(file_contents.str(), "json");
return Document {node, recordLoader};

#ifdef AXOM_USE_HDF5
case Protocol::HDF5:
file_in.close();
conduit::relay::io::load(path, "hdf5", node);
restoreSlashes(node, modifiedNode);
return Document {modifiedNode, recordLoader};
#endif

default:
std::ostringstream message;
message << "Invalid format choice. Please choose from one of the supported "
"protocols: "
<< get_supported_file_types();
throw std::invalid_argument(message.str());
break;
}
}
Expand Down
17 changes: 12 additions & 5 deletions src/axom/sina/core/Document.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,12 @@ namespace sina
enum class Protocol
{
JSON,
#ifdef AXOM_USE_HDF5
HDF5
#endif
};

const std::string supported_types[] = {"JSON",
const std::vector<std::string> supported_types = {"JSON",
#ifdef AXOM_USE_HDF5
"HDF5"
"HDF5"
#endif
};

Expand Down Expand Up @@ -224,6 +222,13 @@ class Document
const std::string &pad = "",
const std::string &eoe = "") const;

/**
* \brief Get the list of file types currently supported by the implementation.
*
* \return a string of supported file types
*/
std::string get_supported_file_types();

private:
/**
* Constructor helper method, extracts info from a conduit Node.
Expand All @@ -237,11 +242,12 @@ class Document
/**
* \brief Save the given Document to the specified location. If the given file exists,
* it will be overwritten.
*
*
* \param document the Document to save
* \param fileName the location of which to save the file
* \param protocol the file type requested to save as contained in supported_types, default = JSON
* \throws std::ios::failure if there are any IO errors
* std::invalid_argument if the protocol given is an undefined, optional protocol
*/
void saveDocument(Document const &document,
std::string const &fileName,
Expand Down Expand Up @@ -276,6 +282,7 @@ Document loadDocument(std::string const &path,
* \param recordLoader the RecordLoader to use to load the different types
* of records
* \param protocol the type of file being loaded, default = JSON
* \throws std::invalid_argument if the protocol given is an undefined, optional protocol
* \return the loaded Document
*/
Document loadDocument(std::string const &path,
Expand Down
20 changes: 15 additions & 5 deletions src/axom/sina/examples/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ set(sina_example_sources
sina_view_datum_values.cpp
)

set(sina_example_depends sina conduit slic)
set(sina_example_depends sina conduit::conduit slic)

if (ENABLE_FORTRAN)
blt_list_append( TO sina_example_sources ELEMENTS sina_fortran.f)
Expand All @@ -38,16 +38,26 @@ endif()
#------------------------------------------------------------------------------
foreach(src ${sina_example_sources})
get_filename_component(exe_name ${src} NAME_WE)

if (exe_name STREQUAL "sina_fortran")
set(sina_mod_src ${CMAKE_CURRENT_BINARY_DIR}/../interface/sina_fortran_interface.f)
else()
set(sina_mod_src "")
endif()

axom_add_executable(
NAME ${exe_name}_ex
SOURCES ${src}
SOURCES ${src} ${sina_mod_src}
OUTPUT_DIR ${EXAMPLE_OUTPUT_DIRECTORY}
DEPENDS_ON ${sina_example_depends}
FOLDER axom/sina/examples
)
)

if (exe_name STREQUAL "sina_fortran")
target_include_directories(${exe_name}_ex PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/../interface)
endif()

# Need to add this flag so XL will ignore trailing underscores in fortran function names
if (${exe_name}_ex STREQUAL "sina_fortran_ex" AND CMAKE_Fortran_COMPILER_ID STREQUAL "XL")
target_compile_options(${exe_name}_ex PRIVATE -qextname)
endif()
endforeach()
endforeach()
Comment on lines 38 to +63
Copy link
Member

Choose a reason for hiding this comment

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

@white238 -- is there a better way in axom/blt to point to the Fortran module?

Our axom_install_component macro already has logic to install the Fortran module.
See:

##------------------------------------------------------------------------------
## axom_install_component
##
## This macro installs libraries, fortran modules, headers, and exports the CMake
## target while preserving the directory stucture. This macro assumes the following:
##
## * CMake Target to install is the same as NAME
## * If there is a Fortran module built, it is named axom_<NAME>.mod
##
## NAME - The name of the component that we are installing.
##
## HEADERS - Headers to be installed
##
##------------------------------------------------------------------------------
macro(axom_install_component)
set(options )
set(singleValueArgs NAME)
set(multiValueArgs HEADERS)
# Parse the arguments to the macro
cmake_parse_arguments(arg
"${options}" "${singleValueArgs}" "${multiValueArgs}" ${ARGN})
set(_header_base_dir include/axom/${arg_NAME})
foreach( _file ${arg_HEADERS} )
get_filename_component( _dir ${_file} DIRECTORY )
install(FILES ${_file} DESTINATION ${_header_base_dir}/${_dir} )
endforeach()
if(ENABLE_FORTRAN)
set(_mod ${CMAKE_Fortran_MODULE_DIRECTORY}/axom_${arg_NAME}.mod)
# TODO: Remove optional once all components have fortran wrappers
install(FILES ${_mod} DESTINATION lib/fortran OPTIONAL)
endif()
endmacro(axom_install_component)

9 changes: 7 additions & 2 deletions src/axom/sina/examples/sina_fortran.f
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
program example
use sina_functions
use hdf5_config
implicit none

! data types
Expand Down Expand Up @@ -57,7 +58,9 @@ program example
full_path = make_cstring(wrk_dir//''//fle_nme)
ofull_path = make_cstring(wrk_dir//''//ofle_nme)
json_fn = make_cstring('sina_dump.json')
hdf5_fn = make_cstring('sina_dump.hdf5')
if (use_hdf5) then
hdf5_fn = make_cstring('sina_dump.hdf5')
end if


mime_type = make_cstring('')
Expand Down Expand Up @@ -151,7 +154,9 @@ program example
! write out the Sina Document
print *,'Writing out the Sina Document'
call write_sina_document(json_fn)
call write_sina_document(hdf5_fn, 1)
if (use_hdf5) then
call write_sina_document(hdf5_fn, 1)
end if


contains
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,9 @@ subroutine save_without_protocol(fname)
call write_sina_document_noprotocol(fname)
end subroutine save_without_protocol

end module
end module

module hdf5_config
implicit none
logical, parameter :: use_hdf5 = @AXOM_USE_HDF5_FORTRAN@
end module hdf5_config
2 changes: 1 addition & 1 deletion src/axom/sina/tests/sina_Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ TEST(Document, saveDocument_json)
EXPECT_EQ("the type", readRecord["type"].as_string());
}

#ifdef AXOM_USE_HDF5
TEST(Document, load_specifiedRecordLoader)
{
using RecordType = TestRecord<int>;
Expand Down Expand Up @@ -463,6 +462,7 @@ TEST(Document, load_defaultRecordLoaders)
EXPECT_NE(nullptr, loadedRun);
}

#ifdef AXOM_USE_HDF5
TEST(Document, create_fromJson_roundtrip_hdf5)
{
std::string orig_json =
Expand Down
Loading