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

Modified cmake projects #155

Merged
merged 22 commits into from
Aug 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
444aa4e
Modify the cmake projects to only allow one library to be built at a …
phlptp Aug 12, 2021
ee18a74
update the cmake for building the test to run with the shared library
phlptp Aug 12, 2021
569c24e
update azure pipelines
phlptp Aug 12, 2021
b88454f
tweak the travis builds
phlptp Aug 12, 2021
b7eb115
try reordering the tests
phlptp Aug 12, 2021
9dc6478
more travis build updates
phlptp Aug 12, 2021
ae61f44
change the header_only target
phlptp Aug 12, 2021
a03421a
Automated formatting of repo files (#156)
github-actions[bot] Aug 12, 2021
6806410
add installer tests
phlptp Aug 13, 2021
2bc410b
more tries at installer test
phlptp Aug 13, 2021
e50ce8d
try a separate config file
phlptp Aug 13, 2021
636880f
add config template
phlptp Aug 13, 2021
5a8e11d
update header_only test case
phlptp Aug 13, 2021
20ddee8
update some more docs
phlptp Aug 13, 2021
d019ed3
fix issue with the install location for built executables
phlptp Aug 13, 2021
676d09f
Automated formatting of repo files (#157)
github-actions[bot] Aug 13, 2021
4fd73d2
add missing includes
phlptp Aug 13, 2021
27328ae
Merge branch 'modified_cmake_projects' of https://github.com/LLNL/uni…
phlptp Aug 13, 2021
487cd04
update include paths and remove some warnings on MSVC
phlptp Aug 14, 2021
f28293d
Automated formatting of repo files (#158)
github-actions[bot] Aug 14, 2021
6b1d26b
update cpplint to ignore more issues that are not relevent
phlptp Aug 14, 2021
7d492f2
Merge branch 'modified_cmake_projects' of https://github.com/LLNL/uni…
phlptp Aug 14, 2021
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
27 changes: 27 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
version: 2
aliases:
- &setup_units
name: setup_units
environment:
command: |
mkdir -p build
cd build
eval cmake .. ${CMAKE_FLAGS}
make -j 4

- &run_units
name: run_units
environment:
Expand All @@ -20,6 +29,13 @@ aliases:
make -j 4
make QUICK_RAW_FUZZ

- &run_installer_tests
name: run_installer_tests
command: |
cd build
make install
ctest -V -R find-package-tests

jobs:
unitsTSan:
docker:
Expand Down Expand Up @@ -69,11 +85,22 @@ jobs:
- store_artifacts:
path: /root/project/build/FuzzTargets/units_fail_measurement_artifact.txt

unitsInstall:
docker:
- image: helics/buildenv:builder
environment:
CMAKE_FLAGS: '-DUNITS_ENABLE_TESTS=ON -DUNITS_INSTALL_PACKAGE_TESTS=ON -DUNITS_BUILD_SHARED_LIBRARY=ON'
steps:
- checkout
- run: *setup_units
- run: *run_installer_tests

workflows:
version: 2
units_test:
jobs:
- unitsMSan
- unitsASan
- unitsTSan
- unitsInstall
- unitsFuzz
27 changes: 23 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ matrix:
- CCACHE_CPP2=yes
script:
- .ci/make_and_test.sh 11
- .ci/make_and_test.sh 14 -DUNITS_HEADER_ONLY=ON
# - .ci/make_and_test.sh 14
# - .ci/make_and_test.sh 17
# Docs and clang 3.5
- compiler: clang
env:
Expand Down Expand Up @@ -65,9 +62,31 @@ matrix:
- '. .ci/build_lcov.sh'
- 'bash .ci/run_codecov.sh'
script:
- .ci/make_and_test.sh 11
- .ci/make_and_test.sh 14 -DUNITS_HEADER_ONLY=ON

# GCC 7 test2 (8 does not support lcov, wait till 9 and new lcov)
- compiler: gcc
env:
- GCC_VER=7
addons:
apt:
sources:
- ubuntu-toolchain-r-test
packages:
- g++-7
- curl
- lcov
install:
- export CC=gcc-7
- export CXX=g++-7
- DEPS_DIR="${TRAVIS_BUILD_DIR}/deps"
- cd $TRAVIS_BUILD_DIR
- '. .ci/build_lcov.sh'
- 'bash .ci/run_codecov.sh'
script:
- .ci/make_and_test.sh 14
- .ci/make_and_test.sh 17
- .ci/make_and_test.sh 11

# GCC 4.8
- compiler: gcc
Expand Down
117 changes: 92 additions & 25 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ cmake_minimum_required(VERSION 3.0)
# Make sure users don't get warnings on a tested (3.0 to 3.16) version of CMake. For
# most of the policies, the new version is better (hence the change). We don't use the
# 3.0...3.17 syntax because of a bug in an older MSVC's built-in and modified CMake 3.11
if(${CMAKE_VERSION} VERSION_LESS 3.18)
if(${CMAKE_VERSION} VERSION_LESS 3.20)
cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION})
else()
cmake_policy(VERSION 3.18)
cmake_policy(VERSION 3.20)
endif()

project(
UNITS
LANGUAGES CXX
VERSION 0.4.1
VERSION 0.5.0
)
include(CMakeDependentOption)
include(CTest)
Expand All @@ -38,6 +38,34 @@ if(NOT DEFINED CMAKE_CXX_STANDARD_REQUIRED)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
endif()

# Set the build output paths
if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
if(NOT CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY
"${CMAKE_BINARY_DIR}/lib"
CACHE PATH "Archive output dir."
)
endif()
if(NOT CMAKE_LIBRARY_OUTPUT_DIRECTORY)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY
"${CMAKE_BINARY_DIR}/lib"
CACHE PATH "Library output dir."
)
endif()
if(NOT CMAKE_PDB_OUTPUT_DIRECTORY)
set(CMAKE_PDB_OUTPUT_DIRECTORY
"${CMAKE_BINARY_DIR}/bin"
CACHE PATH "PDB (MSVC debug symbol)output dir."
)
endif()
if(NOT CMAKE_RUNTIME_OUTPUT_DIRECTORY)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY
"${CMAKE_BINARY_DIR}/bin"
CACHE PATH "Executable/dll output dir."
)
endif()
endif()

list(APPEND CMAKE_MODULE_PATH "${UNITS_SOURCE_DIR}/config")
list(APPEND CMAKE_MODULE_PATH "${UNITS_SOURCE_DIR}/ThirdParty/cmake")

Expand All @@ -55,18 +83,12 @@ set(UNITS_NAMESPACE
CACHE STRING "Top-level namespace name. Default is `units`."
)

option(UNITS_INSTALL "Enable installation of the units library" ON)
mark_as_advanced(UNITS_INSTALL)

cmake_dependent_option(
UNITS_WITH_CMAKE_PACKAGE
"Generate and install cmake package files"
ON
"CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME;UNITS_INSTALL;NOT UNITS_BINARY_ONLY_INSTALL"
OFF
UNITS_INSTALL "Generate and install cmake package files" ON
"CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME;NOT UNITS_BINARY_ONLY_INSTALL" OFF
)

mark_as_advanced(UNITS_WITH_CMAKE_PACKAGE)
mark_as_advanced(UNITS_INSTALL)

cmake_dependent_option(
UNITS_BUILD_FUZZ_TARGETS "Build the targets for a fuzzing system" OFF
Expand All @@ -85,12 +107,8 @@ set(UNITS_CLANG_TIDY_OPTIONS
mark_as_advanced(UNITS_CLANG_TIDY_OPTIONS)
mark_as_advanced(UNITS_CLANG_TIDY)

# Install instructions for this target
if(UNITS_WITH_CMAKE_PACKAGE)
set(UNITS_LIBRARY_EXPORT_COMMAND EXPORT unitsConfig)
else(UNITS_WITH_CMAKE_PACKAGE)
set(UNITS_LIBRARY_EXPORT_COMMAND)
endif(UNITS_WITH_CMAKE_PACKAGE)
set(UNITS_LIBRARY_EXPORT_COMMAND EXPORT unitsTargets)
mark_as_advanced(UNITS_LIBRARY_EXPORT_COMMAND)

option(UNITS_HEADER_ONLY "Expose the units library as header-only" OFF)

Expand Down Expand Up @@ -128,13 +146,39 @@ if(NOT UNITS_HEADER_ONLY)
)
endif(BUILD_SHARED_LIBS)

cmake_dependent_option(
UNITS_BUILD_OBJECT_LIBRARY "Enable construction of the units object library"
OFF "NOT CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME" OFF
else()
option(UNITS_BUILD_STATIC_LIBRARY "enable Construction of the units static library"
OFF
)
option(UNITS_BUILD_SHARED_LIBRARY "enable Construction of the units shared library"
OFF
)

endif(NOT UNITS_HEADER_ONLY)
Copy link
Contributor

@kerim371 kerim371 Aug 13, 2021

Choose a reason for hiding this comment

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

The block:

if(NOT UNITS_HEADER_ONLY)
...
endif(NOT UNITS_HEADER_ONLY)

looks a little hardcoded but I can't propose anything better.

The only thing I can propose is to check whether the developper have checked ON only one option from these three (UNITS_BUILD_STATIC_LIBRARY , UNITS_BUILD_SHARED_LIBRARY, UNITS_HEADER_ONLY) and if there are for example both UNITS_BUILD_STATIC_LIBRARY =ON and UNITS_BUILD_SHARED_LIBRARY=ON we could send a message (WARNING or even FATAL_ERROR) telling that only one library at a time can be built.
Without this it is difficult to predict how the library is going to be built (shared/static/header_only or their combinations) and the developper would need to skim the cmake code to figure that out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added some warnings to check if more than one of UNITS_BUILD_STATIC_LIBRARY, UNITS_BUILD_SHARED_LIBRARY, and UNITS_BUILD_OBJECT_LIBRARY is set, and if you set all three you will get two warnings.


if(UNITS_BUILD_SHARED_LIBRARY AND UNITS_BUILD_STATIC_LIBRARY)
message(
WARNING
"Both UNITS_BUILD_SHARED_LIBRARY and UNITS_BUILD_STATIC_LIBRARY are set to ON, only the shared library will be built"
)
endif()

cmake_dependent_option(
UNITS_BUILD_OBJECT_LIBRARY "Enable construction of the units object library" OFF
"NOT CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME" OFF
)

if(UNITS_BUILD_SHARED_LIBRARY AND UNITS_BUILD_OBJECT_LIBRARY)
message(
WARNING
"Both UNITS_BUILD_SHARED_LIBRARY and UNITS_BUILD_OBJECT_LIBRARY are set to ON, only the shared library will be built"
)
elseif(UNITS_BUILD_STATIC_LIBRARY AND UNITS_BUILD_OBJECT_LIBRARY)
message(
WARNING
"Both UNITS_BUILD_STATIC_LIBRARY and UNITS_BUILD_OBJECT_LIBRARY are set to ON, only the object library will be built"
)
endif()

# Prepare Clang-Tidy
if(UNITS_CLANG_TIDY)
find_program(
Expand Down Expand Up @@ -165,8 +209,31 @@ if(UNITS_INSTALL)
if(UNITS_BUILD_STATIC_LIBRARY)
install(TARGETS compile_flags_target ${UNITS_LIBRARY_EXPORT_COMMAND})
endif()
if(UNITS_WITH_CMAKE_PACKAGE AND NOT UNITS_BINARY_ONLY_INSTALL)
install(EXPORT unitsConfig DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/units)
export(EXPORT unitsConfig)
if(NOT UNITS_BINARY_ONLY_INSTALL)
include(CMakePackageConfigHelpers)
configure_file(
config/unitsConfig.cmake.in "${PROJECT_BINARY_DIR}/unitsConfig.cmake" @ONLY
)

export(
EXPORT unitsTargets
NAMESPACE units::
FILE ${PROJECT_BINARY_DIR}/unitsTargets.cmake
)

install(
EXPORT unitsTargets
NAMESPACE units::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/units
)

write_basic_package_version_file(
phlptp marked this conversation as resolved.
Show resolved Hide resolved
${PROJECT_BINARY_DIR}/unitsConfigVersion.cmake
COMPATIBILITY AnyNewerVersion
)
install(FILES ${PROJECT_BINARY_DIR}/unitsConfigVersion.cmake
${PROJECT_BINARY_DIR}/unitsConfig.cmake
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/units
)
endif()
endif()
1 change: 1 addition & 0 deletions CPPLINT.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ linelength=100 # As in .clang-format

# Non-used filters
filter=-build/include_order # Requires unusual include order that encourages creating not self-contained headers
filter=-build/include_subdir # this is generally good but causes some issues
filter=-readability/nolint # Conflicts with clang-tidy
filter=-runtime/references # Requires fundamental change of API, don't see need for this
filter=-whitespace/blank_line # Unnecessarily strict with blank lines that otherwise help with readability
Expand Down
2 changes: 1 addition & 1 deletion FuzzTargets/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# Copyright (c) 2019-2020,
# Copyright (c) 2019-2021,
# Lawrence Livermore National Security, LLC;
# See the top-level NOTICE for additional details. All rights reserved.
# SPDX-License-Identifier: BSD-3-Clause
Expand Down
2 changes: 1 addition & 1 deletion FuzzTargets/fuzz_target_from_string.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2019-2020,
Copyright (c) 2019-2021,
Lawrence Livermore National Security, LLC;
See the top-level NOTICE for additional details. All rights reserved.
SPDX-License-Identifier: BSD-3-Clause
Expand Down
2 changes: 1 addition & 1 deletion FuzzTargets/fuzz_target_measurement_from_string.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2019-2020,
Copyright (c) 2019-2021,
Lawrence Livermore National Security, LLC;
See the top-level NOTICE for additional details. All rights reserved.
SPDX-License-Identifier: BSD-3-Clause
Expand Down
16 changes: 10 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (!meas.units().is_convertible(out)

## Limitations

- The powers represented by units are limited see [Unit representation](#unit_representation) and only normal physical units or common operations are supported.
- The powers represented by units by default are limited see [Unit representation](#unit_representation) and only normal physical units or common operations are supported, this can be modified at compile time to support a much broader range at the expense of size and computation.
- The library uses floating point and double precision for the multipliers which is generally good enough for most engineering contexts, but does come with the limits and associated loss of precision for long series of calculations on floating point numbers.
- Currency is supported as a unit but it is not recommended to use this for anything beyond basic financial calculations. So if you are doing a lot of financial calculations or accounting use something more specific for currency manipulations.
- Fractional unit powers are not supported in general. While some mathematical operations on units are supported any root operations `sqrt` or `cbrt` will only produce valid results if the result is integral powers of the base units. One exception is limited support for √Hz operations in measurements of Amplitude spectral density. A specific definition of a unit representing square root of Hz is available and will work in combination with other units.
Expand Down Expand Up @@ -99,7 +99,7 @@ These libraries will work well if the number of units being dealt with is known
5. Working with per unit values
6. Dealing with commodities in addition to regular units. i.e. differentiate between a gallon of water and a gallon of gasoline
7. Dealing with equation type units
8. Complete C++ type safety is not a critical design requirement.
8. Complete C++ type safety is **NOT** a critical design requirement.
9. Support is needed for some funky custom unit with bizarre base units.

### Reasons to choose something else
Expand All @@ -108,7 +108,7 @@ These libraries will work well if the number of units being dealt with is known
2. Performance is absolutely critical (many other libraries are zero runtime overhead)
3. You are only working with a small number of known units
4. You cannot use C++11 yet.
5. You need to operate on arbitrary powers of base units
5. You need to operate on arbitrary or fractional powers of base units

## Types

Expand Down Expand Up @@ -143,11 +143,13 @@ The seven [SI units](https://www.nist.gov/pml/weights-and-measures/metric-si/si-

These ranges were chosen to represent nearly all physical quantities that could be found in various disciplines we have encountered.

The CMake variable `UNITS_BASE_TYPE` if set to a 64 bit type like `uint64_t` will double the space requirements but also change the ranges to be at least a power of 4 larger than the above table. See [Cmake Reference](https://units.readthedocs.io/en/latest/installation/cmake_variables.html) for more details.

### Discussion points

- Currency may seem like a unusual choice in units but numbers involving prices are encountered often enough in various disciplines that it is useful to include as part of a unit.
- Technically count and radians are not units, they are representations of real things. A radian is a representation of rotation around a circle and is therefore distinct from a true unitless quantity even though there are no physical measurements associated with either.
- And count and mole are theoretically equivalent though as a practical matter using moles for counts of things is a bit odd for example 1 GB of data is ~1.6605\*10^-15 mol of data. So they are used in different context and don't mix very often, the convert functions does convert between them if necessary.
- Count and mole are theoretically equivalent though as a practical matter using moles for counts of things is a bit odd for example 1 GB of data is ~1.6605\*10^-15 mol of data. So they are used in different context and don't mix very often, the convert functions do convert between them if necessary.
- This library **CANNOT** represent fractional unit powers( except for sqrt Hz used in noise density units), and it follows the order of operation in C++ so **IF** you have equations that any portion of the operation may exceed the numerical limits on powers even if the result does not, **BE CAREFUL**.
- The normal rules about floating point operations losing precision also apply to unit representations with non-integral multipliers.
- With string conversions there are many units that can be interpreted in multiple ways. In general the priority was given to units in more common use in the United States, or in power systems and electrical engineering which was the origin of this library.
Expand All @@ -170,6 +172,8 @@ There are two parts of the library a header only portion that can simply be copi

The second part is a few cpp files that can add some additional functionality. The primary additions from the cpp file are an ability to take roots of units and measurements and convert to and from strings. These files can be built as a standalone static library or included in the source code of whatever project want to use them. The code should build with an C++11 compiler. Most of the library is tagged with constexpr so can be run at compile time to link units that are known at compile time. Unit numerical conversions are not at compile time, so will have a run-time cost. A `quick_convert` function is available to do simple conversions. with a requirement that the units have the same base and not be an equation unit. The cpp code also includes some functions for commodities and will eventually have r20 and x12 conversions, though this is not complete yet.

It builds by default with the static library. Using `UNIT_BUILD_SHARED_LIBRARY` or `BUILD_SHARED_LIBS` will build the shared library instead. Either one can be used with CMake as units::units. The header only library target is also generated `units::header_only`. The shared/static library has a CMake target `units::units`.

## Try it out

If you want to try out the string conversion components. There is server running that can do the string conversions
Expand All @@ -188,7 +192,7 @@ A [converter](https://units.readthedocs.io/en/latest/introduction/converter.html
Many units are defined as `constexpr` objects and can be used directly

```cpp
#include "units.hpp"
#include "units/units.hpp"
using namespace units

measurement length1=45.0*m;
Expand Down Expand Up @@ -275,7 +279,7 @@ These functions are not class methods but operate on units

#### Uncertain measurement methods

Uncertatin measurements have a few additional functions to support the uncertainty calculations
Uncertain measurements have a few additional functions to support the uncertainty calculations

- `rss_add`, `rss_subtract`, `rss_product`, `rss_divide` are equivalent to the associated operator but use the root-sum of squares method for propagating the uncertainty.
- `double uncertainty()` get the numerical value of the uncertainty.
Expand Down
4 changes: 4 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ jobs:
matrix:
Linux14:
vmImage: 'ubuntu-latest'
Linux14shared:
vmImage: 'ubuntu-latest'
units.options: -DUNITS_BUILD_SHARED_LBIRARY=ON
macOS17:
vmImage: 'macOS-latest'
units.std: 17
Expand All @@ -46,6 +49,7 @@ jobs:
units.std: 11
Windows17:
vmImage: 'vs2017-win2016'
units.options: -DUNITS_BUILD_SHARED_LBIRARY=ON
units.std: 17
Windows11:
vmImage: 'vs2017-win2016'
Expand Down
Loading