Skip to content

Commit

Permalink
Change default floating point precision to max (#872)
Browse files Browse the repository at this point in the history 
This changes the behavior of libsdformat so that all floating types emitted to the output stream use maximum precision by default for consistency. A new PrintConfig::SetOutPrecision option is added so that the precision can be configured as desired.

Signed-off-by: Jenn Nguyen <[email protected]>
  • Loading branch information
@jennuine
jennuine authored Apr 13, 2022
1 parent 66c1609 commit 65f0341
Show file tree
Hide file tree
Showing 17 changed files with 506 additions and 209 deletions.
64 changes: 29 additions & 35 deletions include/sdf/Param.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,38 +75,47 @@ namespace sdf
struct ParamStreamer
{
const T &val;
const int precision; // Used to set std::ostream's std::setprecision
explicit ParamStreamer(const T &_val, int _precision = 0)
: val(_val), precision(_precision) {}
};

template<class T> ParamStreamer(T) -> ParamStreamer<T>;

template<class T>
std::ostream& operator<<(std::ostream &os, ParamStreamer<T> s)
{
os << s.val;
return os;
}

template<>
inline std::ostream& operator<<(std::ostream &os, ParamStreamer<double> s)
{
os << std::setprecision(std::numeric_limits<double>::max_digits10) << s.val;
return os;
}
if (s.precision == std::numeric_limits<int>::max())
{
if constexpr (std::is_same_v<T, double>
|| std::is_same_v<T, ignition::math::Angle>
|| std::is_same_v<T, ignition::math::Vector2d>
|| std::is_same_v<T, ignition::math::Vector3d>
|| std::is_same_v<T, ignition::math::Quaterniond>
|| std::is_same_v<T, ignition::math::Pose3d>)
{
os << std::setprecision(std::numeric_limits<double>::max_digits10);
}
else if constexpr (std::is_same_v<T, float>
|| std::is_same_v<T, ignition::math::Color>)
{
os << std::setprecision(std::numeric_limits<float>::max_digits10);
}
}
else
{
os << std::setprecision(s.precision);
}

template<>
inline std::ostream& operator<<(std::ostream &os, ParamStreamer<float> s)
{
os << std::setprecision(std::numeric_limits<float>::max_digits10) << s.val;
os << s.val;
return os;
}

template<class... Ts>
std::ostream& operator<<(std::ostream& os,
ParamStreamer<std::variant<Ts...>> sv)
{
std::visit([&os](auto const &v)
std::visit([&os, &sv](auto const &v)
{
os << ParamStreamer{v};
os << ParamStreamer{v, sv.precision};
}, sv.val);
return os;
}
Expand Down Expand Up @@ -357,7 +366,8 @@ namespace sdf
/// \def ParamVariant
/// \brief Variant type def.
/// Note: When a new variant is added, add variant to functions
/// ParamPrivate::TypeToString and ParamPrivate::ValueFromStringImpl
/// ParamPrivate::TypeToString, ParamPrivate::ValueFromStringImpl,
/// and template std::ostream operator if new variant is floating point
public: typedef std::variant<bool, char, std::string, int, std::uint64_t,
unsigned int, double, float, sdf::Time,
ignition::math::Angle,
Expand Down Expand Up @@ -414,22 +424,6 @@ namespace sdf
const ParamVariant &_value,
std::string &_valueStr) const;

/// \brief Method used to get the string representation from a ParamVariant,
/// or the string that was used to set it.
/// \param[in] _config Print configuration for the string output
/// \param[in] _typeName The data type of the value
/// \param[in] _value The value
/// \param[in] _orignalStr The original string that was used to set the
/// value. A nullopt can be passed in if it is not available.
/// \param[out] _valueStr The output string.
/// \return True if the string was successfully retrieved, false otherwise.
public: bool StringFromValueImpl(
const PrintConfig &_config,
const std::string &_typeName,
const ParamVariant &_value,
const std::optional<std::string> &_originalStr,
std::string &_valueStr) const;

/// \brief Data type to string mapping
/// \return The type as a string, empty string if unknown type
public: template<typename T>
Expand Down
10 changes: 10 additions & 0 deletions include/sdf/PrintConfig.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ namespace sdf
/// False if they are to be expanded.
public: bool PreserveIncludes() const;

/// \brief Set precision of output stream for float / double types.
/// By default, the output stream uses maximum precision.
/// \param[in] _precision The new precision value. To set back to maximum
/// precision, use std::numeric_limits<int>::max().
public: void SetOutPrecision(int _precision);

/// \brief Retrieve the output stream's set precision value.
/// \return The output stream's precision.
public: int OutPrecision() const;

/// \brief Return true if both PrintConfig objects contain the same values.
/// \param[in] _config PrintConfig to compare.
/// \return True if 'this' == _config.
Expand Down
51 changes: 15 additions & 36 deletions src/Param.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,6 @@ std::string Param::GetAsString(const PrintConfig &_config) const
this->dataPtr->StringFromValueImpl(_config,
this->dataPtr->typeName,
this->dataPtr->value,
this->dataPtr->strValue,
valueStr))
{
return valueStr;
Expand All @@ -330,7 +329,6 @@ std::string Param::GetDefaultAsString(const PrintConfig &_config) const
_config,
this->dataPtr->typeName,
this->dataPtr->defaultValue,
this->dataPtr->defaultStrValue,
defaultStr))
{
return defaultStr;
Expand All @@ -339,7 +337,7 @@ std::string Param::GetDefaultAsString(const PrintConfig &_config) const
sdferr << "Unable to get string from default value, "
<< "using ParamStreamer instead.\n";
StringStreamClassicLocale ss;
ss << ParamStreamer{ this->dataPtr->defaultValue };
ss << ParamStreamer{ this->dataPtr->defaultValue, _config.OutPrecision() };
return ss.str();
}

Expand Down Expand Up @@ -797,19 +795,22 @@ bool ParamPrivate::ValueFromStringImpl(const std::string &_typeName,
/// \param[in] _config Printing configuration for the output string.
/// \param[in] _parentAttributes Parent Element Attributes.
/// \param[in] _value The variant value of this pose.
/// \param[in] _originalStr The original string used to set this pose value.
/// \param[out] _valueStr The pose as a string.
/// \return True if the string was successfully retrieved from the pose, false
/// otherwise.
/////////////////////////////////////////////////
bool PoseStringFromValue(const PrintConfig &_config,
const Param_V &_parentAttributes,
const ParamPrivate::ParamVariant &_value,
const std::optional<std::string> &_originalStr,
std::string &_valueStr)
{
StringStreamClassicLocale ss;

if (_config.OutPrecision() == std::numeric_limits<int>::max())
ss << std::setprecision(std::numeric_limits<double>::max_digits10);
else
ss << std::setprecision(_config.OutPrecision());

const ignition::math::Pose3d *pose =
std::get_if<ignition::math::Pose3d>(&_value);
if (!pose)
Expand Down Expand Up @@ -878,7 +879,7 @@ bool PoseStringFromValue(const PrintConfig &_config,
}

// Helper function that sanitizes zero values like '-0'
auto sanitizeZero = [](double _number)
auto sanitizeZero = [&_config](double _number)
{
StringStreamClassicLocale stream;
if (std::fpclassify(_number) == FP_ZERO)
Expand All @@ -887,6 +888,11 @@ bool PoseStringFromValue(const PrintConfig &_config,
}
else
{
if (_config.OutPrecision() == std::numeric_limits<int>::max())
stream << std::setprecision(std::numeric_limits<double>::max_digits10);
else
stream << std::setprecision(_config.OutPrecision());

stream << _number;
}
return stream.str();
Expand Down Expand Up @@ -949,17 +955,6 @@ bool PoseStringFromValue(const PrintConfig &_config,
return true;
}

// If no modification to the value is needed, the original string is returned.
if (!_config.RotationInDegrees() &&
!_config.RotationSnapToDegrees().has_value() &&
!_config.RotationSnapTolerance().has_value() &&
_originalStr.has_value() &&
!_originalStr->empty())
{
_valueStr = _originalStr.value();
return true;
}

ss << pose->Pos() << posRotDelimiter
<< sanitizeZero(pose->Rot().Roll()) << " "
<< sanitizeZero(pose->Rot().Pitch()) << " "
Expand All @@ -974,22 +969,6 @@ bool ParamPrivate::StringFromValueImpl(
const std::string &_typeName,
const ParamVariant &_value,
std::string &_valueStr) const
{
return this->StringFromValueImpl(
_config,
_typeName,
_value,
std::nullopt,
_valueStr);
}

/////////////////////////////////////////////////
bool ParamPrivate::StringFromValueImpl(
const PrintConfig &_config,
const std::string &_typeName,
const ParamVariant &_value,
const std::optional<std::string> &_originalStr,
std::string &_valueStr) const
{
// This will be handled in a type specific manner
if (_typeName == "bool")
Expand All @@ -1012,13 +991,13 @@ bool ParamPrivate::StringFromValueImpl(
if (!this->ignoreParentAttributes && p)
{
return PoseStringFromValue(
_config, p->GetAttributes(), _value, _originalStr, _valueStr);
_config, p->GetAttributes(), _value, _valueStr);
}
return PoseStringFromValue(_config, {}, _value, _originalStr, _valueStr);
return PoseStringFromValue(_config, {}, _value, _valueStr);
}

StringStreamClassicLocale ss;
ss << ParamStreamer{ _value };
ss << ParamStreamer{ _value, _config.OutPrecision() };
_valueStr = ss.str();
return true;
}
Expand Down
19 changes: 18 additions & 1 deletion src/PrintConfig.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*
*/
#include <limits>

#include "sdf/PrintConfig.hh"
#include "sdf/Console.hh"
Expand All @@ -36,6 +37,9 @@ class PrintConfig::Implementation

/// \brief True to preserve <include> tags, false to expand.
public: bool preserveIncludes = false;

/// \brief The output stream's precision. By default, uses maximum precision.
public: int outPrecision = std::numeric_limits<int>::max();
};

/////////////////////////////////////////////////
Expand Down Expand Up @@ -104,13 +108,26 @@ std::optional<double> PrintConfig::RotationSnapTolerance() const
return this->dataPtr->rotationSnapTolerance;
}

/////////////////////////////////////////////////
void PrintConfig::SetOutPrecision(int _precision)
{
this->dataPtr->outPrecision = _precision;
}

/////////////////////////////////////////////////
int PrintConfig::OutPrecision() const
{
return this->dataPtr->outPrecision;
}

/////////////////////////////////////////////////
bool PrintConfig::operator==(const PrintConfig &_config) const
{
if (this->RotationInDegrees() == _config.RotationInDegrees() &&
this->RotationSnapToDegrees() == _config.RotationSnapToDegrees() &&
this->RotationSnapTolerance() == _config.RotationSnapTolerance() &&
this->PreserveIncludes() == _config.PreserveIncludes())
this->PreserveIncludes() == _config.PreserveIncludes() &&
this->OutPrecision() == _config.OutPrecision())
{
return true;
}
Expand Down
13 changes: 13 additions & 0 deletions src/PrintConfig_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,16 @@ TEST(PrintConfig, PreserveIncludes)
config.SetPreserveIncludes(false);
EXPECT_FALSE(config.PreserveIncludes());
}

/////////////////////////////////////////////////
TEST(PrintConfig, OutPrecision)
{
sdf::PrintConfig config;
EXPECT_EQ(std::numeric_limits<int>::max(), config.OutPrecision());
config.SetOutPrecision(2);
EXPECT_EQ(2, config.OutPrecision());
config.SetOutPrecision(8);
EXPECT_EQ(8, config.OutPrecision());
config.SetOutPrecision(std::numeric_limits<int>::max());
EXPECT_EQ(std::numeric_limits<int>::max(), config.OutPrecision());
}
29 changes: 21 additions & 8 deletions src/cmd/cmdsdformat.rb.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@ COMMANDS = { 'sdf' =>
" -d [ --describe ] [SPEC VERSION] Print the aggregated SDFormat spec description. Default version (@SDF_PROTOCOL_VERSION@).\n" +
" -g [ --graph ] <pose, frame> arg Print the PoseRelativeTo or FrameAttachedTo graph. (WARNING: This is for advanced\n" +
" use only and the output may change without any promise of stability)\n" +
" -p [ --print ] arg Print converted arg.\n" +
" -p [ --print ] arg Print converted arg. Note the quaternion representation of the\n" +
" rotational part of poses and unit vectors will be normalized.\n" +
" -i [ --preserve-includes ] Preserve included tags when printing converted arg (does not preserve merge-includes).\n" +
" --degrees Pose rotation angles are printed in degrees.\n" +
" --snap-to-degrees arg Snap pose rotation angles to this specified interval in degrees. This value must be\n" +
" larger than 0, less than or equal to 360, and larger than the defined snap tolerance.\n" +
" --snap-tolerance arg Used in conjunction with --snap-to-degrees, specifies the tolerance at which snapping\n" +
" occurs. This value must be larger than 0, less than 360, and less than the defined\n" +
" degrees value to snap to. If unspecified, its default value is 0.01.\n" +
" --precision arg Set the output stream precision for floating point numbers. The arg must be a positive integer.\n" +

COMMON_OPTIONS
}

Expand All @@ -65,6 +68,7 @@ class Cmd
options = {}
options['degrees'] = 0
options['snap_tolerance'] = 0.01
options['preserve_includes'] = 0

usage = COMMANDS[args[0]]

Expand Down Expand Up @@ -109,6 +113,10 @@ class Cmd
end
options['snap_tolerance'] = arg
end
opts.on('--precision arg', Integer,
'Set the output stream precision for floating point numbers.') do |arg|
options['precision'] = arg
end
opts.on('-g arg', '--graph type', String,
'Print PoseRelativeTo or FrameAttachedTo graph') do |graph_type|
options['graph'] = {:type => graph_type}
Expand All @@ -131,7 +139,8 @@ class Cmd

options['command'] = ARGV[0]

if options['preserve_includes'] and not options['print']
if (options['preserve_includes'] != 0 and not options['print']) ||
(options['precision'] and not options['print'])
puts usage
exit(-1)
end
Expand Down Expand Up @@ -206,21 +215,25 @@ class Cmd
exit(Importer.cmdDescribe(options['describe']))
elsif options.key?('print')
snap_to_degrees = 0
if options['preserve_includes']
Importer.extern 'int cmdPrintPreserveIncludes(const char *)'
exit(Importer.cmdPrintPreserveIncludes(File.expand_path(options['print'])))
elsif options.key?('snap_to_degrees')
precision = 0

if options.key?('snap_to_degrees')
if options['snap_to_degrees'] < options['snap_tolerance']
puts "Rotation snapping tolerance must be larger than the snapping tolerance."
exit(-1)
end
snap_to_degrees = options['snap_to_degrees']
end
Importer.extern 'int cmdPrint(const char *, int in_degrees, int snap_to_degrees, float snap_tolerance)'
if options.key?('precision')
precision = options['precision']
end
Importer.extern 'int cmdPrint(const char *, int in_degrees, int snap_to_degrees, float snap_tolerance, int, int)'
exit(Importer.cmdPrint(File.expand_path(options['print']),
options['degrees'],
snap_to_degrees,
options['snap_tolerance']))
options['snap_tolerance'],
options['preserve_includes'],
precision))
elsif options.key?('graph')
Importer.extern 'int cmdGraph(const char *, const char *)'
exit(Importer.cmdGraph(options['graph'][:type], File.expand_path(ARGV[1])))
Expand Down
Loading

0 comments on commit 65f0341

Please sign in to comment.