-
Notifications
You must be signed in to change notification settings - Fork 102
feat: add limits to input attributes #4063
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
Open
kdrienCG
wants to merge
20
commits into
develop
Choose a base branch
from
feature/kdrienCG/attributeLimitMetadata
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
2f08163
(WIP) add min/max limit to Wrapper.hpp
kdrienCG 45713ea
move min/max optionals in a struct instanciated only when T is limitable
kdrienCG c07a75a
create is_limitable trait
kdrienCG 4c49318
add limits validation for input values
kdrienCG 19d2ae8
add limits modes to limits
kdrienCG fd14de4
implement limits modes in validateLimits()
kdrienCG 32f1586
remove setMinValue() and setMaxValue()
kdrienCG 796bb16
add convenience alias is_limitable_v
kdrienCG 14bda7b
modify outside range message
kdrienCG 581b6d8
add inclusive-exclusive properties using a Bound type
kdrienCG 2590521
Merge branch 'develop' into feature/kdrienCG/attributeLimitMetadata
kdrienCG 7436f3c
support arrays of numbers
kdrienCG a456aca
set validateLimit() to public
kdrienCG 80a9541
add tests
kdrienCG 65779e4
add getDataContext() to the limit validation message
kdrienCG 54053c7
add range to the limit validation message
kdrienCG 7d08bd2
set default mode to Error
kdrienCG ace25d5
move range string creation in the Limits struct
kdrienCG 0f51019
add limits to generated documentation
kdrienCG 3ccdc0f
add GEOS_UNUSED_PARAM to setLimits overload for non-limits
kdrienCG File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| /* | ||
| * ------------------------------------------------------------------------------------------------------------ | ||
| * SPDX-License-Identifier: LGPL-2.1-only | ||
| * | ||
| * Copyright (c) 2016-2024 Lawrence Livermore National Security LLC | ||
| * Copyright (c) 2018-2024 TotalEnergies | ||
| * Copyright (c) 2018-2024 The Board of Trustees of the Leland Stanford Junior University | ||
| * Copyright (c) 2023-2024 Chevron | ||
| * Copyright (c) 2019- GEOS/GEOSX Contributors | ||
| * All rights reserved | ||
| * | ||
| * See top level LICENSE, COPYRIGHT, CONTRIBUTORS, NOTICE, and ACKNOWLEDGEMENTS files for details. | ||
| * ------------------------------------------------------------------------------------------------------------ | ||
| */ | ||
|
|
||
|
|
||
| #ifndef GEOS_DATAREPOSITORY_ATTRIBUTELIMITS_HPP_ | ||
| #define GEOS_DATAREPOSITORY_ATTRIBUTELIMITS_HPP_ | ||
|
|
||
| #include "common/DataTypes.hpp" | ||
| #include "common/format/EnumStrings.hpp" | ||
| #include <optional> | ||
| #include <type_traits> | ||
|
|
||
| namespace geos | ||
| { | ||
|
|
||
| namespace dataRepository | ||
| { | ||
|
|
||
| /** | ||
| * @enum LimitsMode | ||
| * @brief Enforcement mode associated with the limits of an attribute | ||
| * | ||
| * - Indicative: the limits are documentation only, no runtime check is performed. | ||
| * - Warning: a value outside the limits emits a runtime warning. | ||
| * - Error: a value outside the limits throws. | ||
| */ | ||
| enum class LimitsMode : integer | ||
| { | ||
| Indicative, | ||
| Warning, | ||
| Error | ||
| }; | ||
|
|
||
| ENUM_STRINGS( LimitsMode, | ||
| "Indicative", | ||
| "Warning", | ||
| "Error" ); | ||
|
|
||
| /** | ||
| * @struct is_limitable | ||
| * @tparam T type to check | ||
| * @brief Trait determining whether attribute limits can be applied to type @p T | ||
| * | ||
| * Limits apply to scalar numeric types (integer, real32, real64, etc.) | ||
| */ | ||
| template< typename T > | ||
| struct is_limitable | ||
| { | ||
| static constexpr bool value = std::is_arithmetic< T >::value; | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Convenience variable template alias for is_limitable< T >::value | ||
| */ | ||
| template< typename T > | ||
| inline constexpr bool is_limitable_v = is_limitable< T >::value; | ||
|
|
||
| /** | ||
| * @struct Bound | ||
| * @brief Structure containing informations about an attribute limit. | ||
| */ | ||
| template< typename T > | ||
| struct Bound | ||
| { | ||
| T value; | ||
| bool isInclusive = true; | ||
|
|
||
| /** | ||
| * @brief Bound constructor to write a limit without the "Bound{ ... }" syntax | ||
| * @param value The limit value to set | ||
| * @param isInclusive Wether the limit should be inclusive or not | ||
| * | ||
| * @code | ||
| * .setLimits( 0.0, 1.0 ) // where setLimits takes `Bound` parameters, those parameters can | ||
| * // be written only with the value. The isInclusive property will | ||
| * // default to true. | ||
| * @endcode | ||
| */ | ||
| Bound( T v, bool inclusive = true ) | ||
| : value( v ), isInclusive( inclusive ) | ||
| {} | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Creates an inclusive limit of @p value | ||
| * @param value The inclusive limit value to set | ||
| */ | ||
| template< typename T > | ||
| Bound< T > inclusive( T value ) | ||
| { | ||
| return Bound< T >{ value, /*isInclusive*/ true }; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Creates an exclusive limit of @p value | ||
| * @param value The inclusive limit value to set | ||
| */ | ||
| template< typename T > | ||
| Bound< T > exclusive( T value ) | ||
| { | ||
| return Bound< T >{ value, /*isInclusive*/ false }; | ||
| } | ||
|
|
||
| /** | ||
| * @struct Limits | ||
| * @brief Storage for the optional min/max bounds of a wrapped value. | ||
| * | ||
| * Specialized so that the members (std::optional< T >) are only instanciated | ||
| * for limitable types. Preventing instantiation of non-limitable types, especially | ||
| * abstract types that can't be instantiated with std::optional< absT >. | ||
| */ | ||
| template< typename T, bool = is_limitable_v< T > > | ||
| struct Limits | ||
| {}; | ||
|
|
||
| template< typename T > | ||
| struct Limits< T, true > | ||
| { | ||
| std::optional< Bound< T > > min; | ||
| std::optional< Bound< T > > max; | ||
| }; | ||
|
|
||
|
|
||
| // Helper methods | ||
|
|
||
| /** | ||
| * @brief Compare the given value with the min limit, taking account for the inclusive xor exclusive | ||
| * property of the limit. | ||
| * @param value The value to compare to the limit | ||
| * @param minLimit The min limit containing the inclusive | ||
| * @return True if the value is below the min limit, false otherwise | ||
| */ | ||
| template< typename T > | ||
| static bool isValueBelowMin( T const & value, Bound< T > const & minLimit ) | ||
| { | ||
| return minLimit.isInclusive ? ( value < minLimit.value ) | ||
| : ( value <= minLimit.value ); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Compare the given value with the max limit, taking account for the inclusive xor exclusive | ||
| * property of the limit. | ||
| * @param value The value to compare to the limit | ||
| * @param maxLimit The max limit containing the inclusive | ||
| * @return True if the value is above the max limit, false otherwise | ||
| */ | ||
| template< typename T > | ||
| static bool isValueAboveMax( T const & value, Bound< T > const & maxLimit ) | ||
| { | ||
| return maxLimit.isInclusive ? ( value > maxLimit.value ) | ||
| : ( value >= maxLimit.value ); | ||
| } | ||
|
|
||
| } /* namespace dataRepository */ | ||
|
|
||
| } /* namespace geos */ | ||
|
|
||
| #endif /* GEOS_DATAREPOSITORY_ATTRIBUTELIMITS_HPP_ */ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,9 @@ | |
| #define GEOS_DATAREPOSITORY_WRAPPER_HPP_ | ||
|
|
||
| // Source inclues | ||
| #include "common/format/Format.hpp" | ||
| #include "common/logger/Logger.hpp" | ||
| #include "dataRepository/AttributeLimits.hpp" | ||
| #include "wrapperHelpers.hpp" | ||
| #include "KeyNames.hpp" | ||
| #include "LvArray/src/limits.hpp" | ||
|
|
@@ -37,8 +40,8 @@ | |
| #include "WrapperBase.hpp" | ||
|
|
||
| // System includes | ||
| #include <type_traits> | ||
| #include <cstdlib> | ||
| #include <optional> | ||
| #include <type_traits> | ||
|
|
||
| namespace geos | ||
|
|
@@ -204,6 +207,7 @@ class Wrapper final : public WrapperBase | |
| m_ownsData = castedSource.m_ownsData; | ||
| m_default = castedSource.m_default; | ||
| m_dimLabels = castedSource.m_dimLabels; | ||
| m_limits = castedSource.m_limits; | ||
| } | ||
|
|
||
| /////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
@@ -718,6 +722,71 @@ class Wrapper final : public WrapperBase | |
| return ss.str(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Set both bounds for this attribute's value. | ||
| * @param min the minimum allowed value (inclusive) | ||
| * @param max the maximum allowed value (inclusive) | ||
| * @param mode the enforcement mode | ||
| * @return pointer to Wrapper<T> | ||
| * | ||
| * @note @p min and @p max are std::optional(s). | ||
| * Set them to std::nullopt to disable a limit. | ||
| * | ||
| * @code | ||
| * registerWrapper( viewKeysStruct::fooString(), &m_foo ) | ||
| * .setLimits( 0.0, 1.0 ) // sets a minimum value of 0.0 and a maximum value of 1.0 | ||
| * .setLimits( 0.0, std::nullopt ) // sets a minimum value of 0.0 and no maximum value | ||
| * .setLimits( inclusive( 0.0 ), std::nullopt ) // sets an inclusive (default) minimum value | ||
| * .setLimits( exclusive( 0.0 ), std::nullopt ) // sets an exclusive maximum value | ||
| * @endcode | ||
| */ | ||
| template< typename U=T > | ||
| std::enable_if_t< is_limitable_v< U >, Wrapper< T > & > | ||
| setLimits( std::optional< Bound< T > > min, | ||
| std::optional< Bound< T > > max, | ||
| LimitsMode mode = LimitsMode::Warning ) | ||
| { | ||
| m_limits.min = min; | ||
| m_limits.max = max; | ||
| m_limitsMode = mode; | ||
| return *this; | ||
| } | ||
|
|
||
| template< typename U=T > | ||
| std::enable_if_t< !is_limitable_v< U >, Wrapper< T > & > | ||
| setLimits( std::optional< Bound< T > >, | ||
| std::optional< Bound< T > >, | ||
| LimitsMode mode = LimitsMode::Warning ) | ||
| { | ||
| static_assert( is_limitable_v< U >, | ||
| "setLimits is only supported on scalar arithmetic types." ); | ||
| return *this; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Accessor for the minimum bound of this attribute's value. | ||
| * @return optional containing the typed minimum value, empty if not set | ||
| * @note Only available when T is a limitable type | ||
| */ | ||
| template< typename U=T > | ||
| std::enable_if_t< is_limitable_v< U >, std::optional< Bound< T > > const & > | ||
| getMinValue() const | ||
| { | ||
| return m_limits.min; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Accessor for the maximum bound of this attribute's value. | ||
| * @return optional containing the typed maximum value, empty if not set | ||
| * @note Only available when T is a limitable type | ||
| */ | ||
| template< typename U=T > | ||
| std::enable_if_t< is_limitable_v< U >, std::optional< Bound< T > > const & > | ||
| getMaxValue() const | ||
| { | ||
| return m_limits.max; | ||
| } | ||
|
|
||
| virtual bool processInputFile( xmlWrapper::xmlNode const & targetNode, | ||
| xmlWrapper::xmlNodePos const & nodePos ) override | ||
| { | ||
|
|
@@ -749,6 +818,11 @@ class Wrapper final : public WrapperBase | |
| targetNode, | ||
| getDefaultValueStruct() ); | ||
| } | ||
|
|
||
| if( m_successfulReadFromInput ) | ||
| { | ||
| validateLimits(); | ||
| } | ||
| } | ||
| catch( std::exception const & ex ) | ||
| { | ||
|
|
@@ -1086,6 +1160,50 @@ class Wrapper final : public WrapperBase | |
| return this->packByIndexImpl< false >( dummy, packList, withMetadata, onDevice, events ); | ||
| } | ||
|
|
||
| template< typename U=T > | ||
| std::enable_if_t< is_limitable_v< U >, void > | ||
| validateLimits() | ||
| { | ||
| if( (!m_limits.min.has_value() && !m_limits.max.has_value()) || | ||
| m_limitsMode == LimitsMode::Indicative ) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| T const & value = reference(); | ||
| bool const belowMin = m_limits.min.has_value() ? isValueBelowMin( value, *m_limits.min ) : false; | ||
| bool const aboveMax = m_limits.max.has_value() ? isValueAboveMax( value, *m_limits.max ) : false; | ||
| if( !belowMin && !aboveMax ) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| string const msg = GEOS_FMT( "Value {} for attribute '{}' is outside the allowed range.", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add the range to the message. Also will this show the full context, not just the field name? Should we use |
||
| value, getName() ); | ||
|
|
||
| switch( m_limitsMode ) | ||
| { | ||
| case LimitsMode::Warning: | ||
| GEOS_WARNING( msg ); | ||
| break; | ||
|
|
||
| case LimitsMode::Error: | ||
| GEOS_THROW( msg, InputError ); | ||
| break; | ||
|
|
||
| default: | ||
| GEOS_LOG_RANK_0( "Unimplemented LimitsMode" ); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| template< typename U=T > | ||
| std::enable_if_t< !is_limitable_v< U >, void > | ||
| validateLimits() | ||
| { | ||
| /* no-op */ | ||
| } | ||
|
|
||
| /// flag to indicate whether or not this wrapper is responsible for allocation/deallocation of the object at the | ||
| /// address of m_data | ||
| bool m_ownsData; | ||
|
|
@@ -1100,6 +1218,9 @@ class Wrapper final : public WrapperBase | |
|
|
||
| /// stores dimension labels (used mainly for plotting) for multidimensional arrays, empty member otherwise | ||
| wrapperHelpers::ArrayDimLabels< T > m_dimLabels; | ||
|
|
||
| /// stores the (optional) min/max bounds for the wrapped value. | ||
| Limits< T > m_limits; | ||
| }; | ||
|
|
||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.