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

Add C wrapper to basix for tabulation in C-code #612

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
9 changes: 6 additions & 3 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 3.16)

# Set the version
project(Basix VERSION "0.5.2.0" LANGUAGES CXX)
project(Basix VERSION "0.5.2.0" LANGUAGES CXX C)
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? I'm thinking the build for Basix will still be CXX. The downstream project would use C.

https://cmake.org/cmake/help/latest/command/project.html

include(GNUInstallDirs)

if(SKBUILD)
Expand Down Expand Up @@ -62,7 +62,8 @@ set(HEADERS_basix
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-hermite.h
${CMAKE_CURRENT_SOURCE_DIR}/basix/mdspan.hpp
${CMAKE_CURRENT_SOURCE_DIR}/basix/docs.h
${CMAKE_CURRENT_BINARY_DIR}/basix/version.h)
${CMAKE_CURRENT_BINARY_DIR}/basix/version.h
${CMAKE_CURRENT_SOURCE_DIR}/basix/cwrapper.h)

target_sources(basix PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/basix/cell.cpp
Expand All @@ -87,10 +88,12 @@ target_sources(basix PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-crouzeix-raviart.cpp
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-bubble.cpp
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-hermite.cpp
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-serendipity.cpp)
${CMAKE_CURRENT_SOURCE_DIR}/basix/e-serendipity.cpp
${CMAKE_CURRENT_SOURCE_DIR}/basix/cwrapper.cpp)

# Configure the library
set_target_properties(basix PROPERTIES PUBLIC_HEADER basix/finite-element.h)
set_target_properties(basix PROPERTIES PUBLIC_HEADER basix/cwrapper.h)
set_target_properties(basix PROPERTIES PRIVATE_HEADER "${HEADERS_basix}")
target_include_directories(basix PUBLIC
$<INSTALL_INTERFACE:include>
Expand Down
61 changes: 61 additions & 0 deletions cpp/basix/cwrapper.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) 2022 Susanne Claus
Copy link
Member

Choose a reason for hiding this comment

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

We use clang-format on our C++ files.

// FEniCS Project
// SPDX-License-Identifier: MIT

// This file defines the C-API functions listed in cwrapper.h
// This is a C++ file that defines C calls
#include <basix/finite-element.h>
#include <basix/cell.h>
#include <span>
#include "cwrapper.h"

extern "C" {

basix_element* basix_element_create(const int basix_family, const int basix_cell_type, const int degree,
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 using int and then casting to the basix enum is OK, but I think we should explicitly add the int datatype to the class enums in basix, e.g.:

https://github.com/FEniCS/basix/blob/main/cpp/basix/element-families.h

https://en.cppreference.com/w/cpp/language/enum

"Values of integer, floating-point, and enumeration types can be converted by static_cast or explicit cast, to any enumeration type. If the underlying type is not fixed and the source value is out of range, the behavior is undefined."

Copy link
Member

Choose a reason for hiding this comment

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

A good alternative would be to have C enum equivalents of the existing C++ enums, as mentioned above.

const int lagrange_variant, const int dpc_variant, const bool discontinuous)
{
//Specify which element is needed
basix::element::family family = static_cast<basix::element::family>(basix_family);
basix::cell::type cell_type = static_cast<basix::cell::type>(basix_cell_type);
Copy link
Member

Choose a reason for hiding this comment

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

reinterpret_cast might work?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly not a good idea

int k = degree;
basix::element::lagrange_variant lvariant = static_cast<basix::element::lagrange_variant>(lagrange_variant);
Copy link
Member

Choose a reason for hiding this comment

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

lvariant etc. should also be const.

basix::element::dpc_variant dvariant = static_cast<basix::element::dpc_variant>(dpc_variant);

//Create C++ basix element object
basix::FiniteElement finite_element = basix::create_element(family, cell_type, k, lvariant,dvariant,discontinuous);

basix_element* element = reinterpret_cast<basix_element*>(new basix::FiniteElement(finite_element));

return element;
}

void basix_element_destroy(basix_element *element)
{
delete reinterpret_cast<basix::FiniteElement*>(element);
}

void basix_element_tabulate_shape(basix_element *element, const unsigned int num_points,
const int nd, int* cshape)
{
//Determine shape of tabulated values to allocate sufficient memory
std::array<std::size_t, 4> shape = reinterpret_cast<basix::FiniteElement*>(element)->tabulate_shape(nd, num_points);

//Copy shape values
std::copy(shape.begin(),shape.end(),cshape);
}

void basix_element_tabulate(basix_element *element, const unsigned int gdim, const double* points,
const unsigned int num_points, const int nd,
double* table_data, long unsigned int table_data_size)
{
basix::FiniteElement* finite_element = reinterpret_cast<basix::FiniteElement*>(element);

//Create span views on points and table values
std::span<const double> points_view{points,num_points*gdim};
std::span<double> basis{table_data, table_data_size};
std::array<std::size_t, 2> xshape{num_points,gdim};

finite_element->tabulate(nd, points_view, xshape, basis);
}

}
43 changes: 43 additions & 0 deletions cpp/basix/cwrapper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) 2022 Susanne Claus
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an example of use in e.g. demo/c/wrapper?

That's often helpful for getting the API looking right.

// FEniCS Project
// SPDX-License-Identifier: MIT

// Declares a simple C API wrapper around C++.

#ifndef CWRAPPER_H_
#define CWRAPPER_H_

#include <stdbool.h>

#ifdef __cplusplus
// extern C indicates to C++ compiler that the following code
// is meant to be called by a C compiler
// i.e. tells the compiler that C naming conventions etc should be used
extern "C" {
#endif

//@todo Could this be replaced by ufcx_finite_element?
/// Basix element structure to map c basix element to C++ basix finite_element
/// The following int values indicate the position of the values in the corresponding
/// enum classes in basix
typedef struct basix_element basix_element;

basix_element* basix_element_create(const int basix_family, const int basix_cell_type, const int degree,
Copy link
Member

Choose a reason for hiding this comment

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

It's super tedious, but I think to make this interface useable you will need to recreate the C++ enums as C enums.

const int lagrange_variant, const int dpc_variant, const bool discontinuous);

void basix_element_destroy(basix_element *element);

void basix_element_tabulate_shape(basix_element *element, const unsigned int num_points,
const int nd, int* cshape);

void basix_element_tabulate(basix_element *element, const unsigned int gdim, const double* points,
const unsigned int num_points, const int nd,
double* table_data, long unsigned int table_data_size);


#ifdef __cplusplus
}
#endif


#endif /*CWRAPPER_H_*/