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.

Sign up for GitHub

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

Jump to bottom

fixes for builds with no raja or umpire #1270

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
+41 −46
Open

fixes for builds with no raja or umpire #1270

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
55816b5
fixes for builds with no raja
ebchin Nov 12, 2024
12debca
change raja to umpire where appropriate
ebchin Nov 12, 2024
a07d20c
use dynamic memory for all
ebchin Nov 14, 2024
1ccf12d
Merge branch 'develop' into bugfix/chin23/build_no_raja
ebchin Nov 14, 2024
b132020
copy arr if needed
ebchin Nov 15, 2024
2a9a146
Merge branch 'develop' into bugfix/chin23/build_no_raja
ebchin Nov 15, 2024
3028760
separate if statements
ebchin Nov 15, 2024
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 src/serac/infrastructure/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ set(infrastructure_sources
terminator.cpp
)

set(infrastructure_depends axom::inlet axom::fmt axom::cli11 mfem ${serac_device_depends})
set(infrastructure_depends axom::inlet axom::fmt axom::cli11 camp mfem ${serac_device_depends})
blt_list_append(TO infrastructure_depends ELEMENTS tribol IF TRIBOL_FOUND)
blt_list_append(TO infrastructure_depends ELEMENTS caliper adiak::adiak IF SERAC_ENABLE_PROFILING)
list(APPEND infrastructure_depends blt::mpi)
Expand Down
8 changes: 8 additions & 0 deletions src/serac/infrastructure/debug_print.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ std::ostream& operator<<(std::ostream& out, serac::SignedIndex i)
* @param filename the name of the output file
*/
template <typename T>
#ifdef SERAC_USE_RAJA
void write_to_file(axom::Array<T, 2, axom::MemorySpace::Host> arr, std::string filename)
#else
Copy link
Member

Choose a reason for hiding this comment

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

You would think that Host would always be there but its guarded in axom and Dynamic is the one unguarded....

https://github.com/LLNL/axom/blob/2244835b3389c427c337b5df782982d2ae609bfe/src/axom/core/memory_management.hpp#L37-L47

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like host should be safe to unguard in addition to dynamic? Regardless, we could use dynamic + umpire allocator IDs to achieve the same without the clunky #ifdef blocks. Though we would lose the explicit typing of the memory. Thoughts?

Copy link
Member

@white238 white238 Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I say go with Dynamic for now to avoid the ifdef's... but this really needs to be revisited. Might need to chat with Axom about this.

Copy link
Member

Choose a reason for hiding this comment

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

@white238 and I spoke -- it makes sense to go with (the default) Dynamic

Copy link
Member Author

@ebchin ebchin Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's not clear to me Dynamic will give the desired result when the original axom::Array is on device. It seems like this copy constructor will be called: https://github.com/LLNL/axom/blob/develop/src/axom/core/Array.hpp#L1180-L1190 which copies the allocator ID from the existing array. The result will be an array on device and the operator() call on L88 will fail. Is there a different copy constructor being called by this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented a fix suggested by @kennyweiss

void write_to_file(axom::Array<T, 2> arr, std::string filename)
#endif
{
std::ofstream outfile(filename);

Expand All @@ -97,7 +101,11 @@ void write_to_file(axom::Array<T, 2, axom::MemorySpace::Host> arr, std::string f
* @param filename the name of the output file
*/
template <typename T>
#ifdef SERAC_USE_RAJA
void write_to_file(axom::Array<T, 3, axom::MemorySpace::Host> arr, std::string filename)
#else
void write_to_file(axom::Array<T, 3> arr, std::string filename)
#endif
{
std::ofstream outfile(filename);

Expand Down
2 changes: 2 additions & 0 deletions src/serac/numerics/functional/domain_integral_kernels.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include "serac/numerics/functional/quadrature_data.hpp"
#include "serac/numerics/functional/function_signature.hpp"
#include "serac/numerics/functional/differentiate_wrt.hpp"
#ifdef SERAC_USE_RAJA
#include "RAJA/RAJA.hpp"
#endif

#include <array>
#include <cstdint>
Expand Down
36 changes: 30 additions & 6 deletions src/serac/numerics/functional/element_restriction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,12 @@ std::vector<Array2D<int> > geom_local_face_dofs(int p)
return output;
}

axom::Array<DoF, 2, axom::MemorySpace::Host> GetElementRestriction(const mfem::FiniteElementSpace* fes,
mfem::Geometry::Type geom)
#ifdef SERAC_USE_RAJA
axom::Array<DoF, 2, axom::MemorySpace::Host>
#else
axom::Array<DoF, 2>
#endif
GetElementRestriction(const mfem::FiniteElementSpace* fes, mfem::Geometry::Type geom)
{
std::vector<DoF> elem_dofs{};
mfem::Mesh* mesh = fes->GetMesh();
Expand Down Expand Up @@ -267,17 +271,29 @@ axom::Array<DoF, 2, axom::MemorySpace::Host> GetElementRestriction(const mfem::F
}

if (n == 0) {
#ifdef SERAC_USE_RAJA
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be SERAC_USE_UMPIRE

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah makes sense, basically all of the axom::MemorySpace::Host stuff needs to be fixed.

return axom::Array<DoF, 2, axom::MemorySpace::Host>{};
#else
return axom::Array<DoF, 2>{};
#endif
} else {
uint64_t dofs_per_elem = elem_dofs.size() / n;
uint64_t dofs_per_elem = elem_dofs.size() / n;
#ifdef SERAC_USE_RAJA
axom::Array<DoF, 2, axom::MemorySpace::Host> output(n, dofs_per_elem);
#else
axom::Array<DoF, 2> output(n, dofs_per_elem);
#endif
std::memcpy(output.data(), elem_dofs.data(), sizeof(DoF) * n * dofs_per_elem);
return output;
}
}

axom::Array<DoF, 2, axom::MemorySpace::Host> GetFaceDofs(const mfem::FiniteElementSpace* fes,
mfem::Geometry::Type face_geom, FaceType type)
#ifdef SERAC_USE_RAJA
axom::Array<DoF, 2, axom::MemorySpace::Host>
#else
axom::Array<DoF, 2>
#endif
GetFaceDofs(const mfem::FiniteElementSpace* fes, mfem::Geometry::Type face_geom, FaceType type)
{
std::vector<DoF> face_dofs;
mfem::Mesh* mesh = fes->GetMesh();
Expand Down Expand Up @@ -378,10 +394,18 @@ axom::Array<DoF, 2, axom::MemorySpace::Host> GetFaceDofs(const mfem::FiniteEleme
delete face_to_elem;

if (n == 0) {
#ifdef SERAC_USE_RAJA
return axom::Array<DoF, 2, axom::MemorySpace::Host>{};
#else
return axom::Array<DoF, 2>{};
#endif
} else {
uint64_t dofs_per_face = face_dofs.size() / n;
uint64_t dofs_per_face = face_dofs.size() / n;
#ifdef SERAC_USE_RAJA
axom::Array<DoF, 2, axom::MemorySpace::Host> output(n, dofs_per_face);
#else
axom::Array<DoF, 2> output(n, dofs_per_face);
#endif
std::memcpy(output.data(), face_dofs.data(), sizeof(DoF) * n * dofs_per_face);
return output;
}
Expand Down
4 changes: 4 additions & 0 deletions src/serac/numerics/functional/element_restriction.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,11 @@ struct ElementRestriction {
uint64_t nodes_per_elem;

/// a 2D array (num_elements-by-nodes_per_elem) holding the dof info extracted from the finite element space
#ifdef SERAC_USE_RAJA
axom::Array<DoF, 2, axom::MemorySpace::Host> dof_info;
#else
axom::Array<DoF, 2> dof_info;
#endif

/// whether the underlying dofs are arranged "byNodes" or "byVDim"
mfem::Ordering::Type ordering;
Expand Down