IFCOS C++ Internals - Debugging Assertions and Numeric Tolerances

[RickBrice]

As I work on adding support for alignments and linear referencing, I am making assumptions about IFC and I want to add assertions so there is automatic notification if the assumptions are incorrect. What is the preferred way to handle this?
I can use the traditional assert(value == expected_value); approach. But it seems that there is a Logger with Error and Warnings. I recall seeing std::runtime_error exceptions being thrown in a few places as well.

Some examples

IfcOpenShell/src/ifcgeom/mapping/IfcCurveSegment.cpp

Lines 256 to 259 in 164125f

	if (p->size() < 2)
	{
	throw std::runtime_error("invalid polyline - must have at least 2 points"); // this should never happen, but just in case it does
	}

IfcPolyline must have at least 2 points – is throwing a std::runtime_error the best thing to do?

I expect the points defining the IfcPolyline to be 2D when used as the basis curve for IfcCurveSegment. I’m not 100% sure this is a requirement of IFC, but it is an assumption I’m making. If we encounter an IFC file that uses an IfcPolyline with 3D points, I’d like an assertion to fire.

IfcOpenShell/src/ifcgeom/mapping/IfcCurveSegment.cpp

Line 267 in 164125f

auto p1 = *(iter++);

Is assert(p1->Coordinates().size() == 2) ok or should the error logger be used?

Alternatively, IfcCurve has a Dim attribute. That could be evaluated, but I don’t know how to get the value. Please point me to an example of how I can get the Dim attribute

Lastly, what is the preferred method for dealing with numerical tolerances?
In this example, checking that l is exactly 0.0 is ok, but a tolerance could be applied

IfcOpenShell/src/ifcgeom/mapping/IfcCurveSegment.cpp

Lines 281 to 286 in 164125f

	auto l = sqrt(dx * dx + dy * dy);
	if (l == 0.0)
	{
	// @todo use closeness tolerance instead of absolute 0.0
	throw std::runtime_error("invalid polyline - points must not be coincident");
	}

In this example, I’ve run into cases where the last span is slightly shorter than the last value of u so I changed to if(u < length+0.001) (this change not committed yet). Is there a more rational and consistent way to specify the tolerance instead of my arbitrary value of 0.001?

IfcOpenShell/src/ifcgeom/taxonomy.h

Lines 153 to 161 in 164125f

	Eigen::VectorXd evaluate(double u) const {
	// @todo optimize, assume monotonic evaluation and store last evaluated segment?
	for (auto& [length, fn] : spans) {
	if (u < length) {
	return fn(u);
	}
	u -= length;
	}
	}

[aothms]

I already made comment on the PR before seeing that. We indeed have Logger::Warning, Logger::Error or raise an exception. I would not use assert() because typically in release builds they simply disappear or generally don't result in a nice experience (especially for windows users, I think on linux you get a bit more informative output from them as an enduser).

I would say:

• raise exception. no point in continuing evaluation of this instance, depending on how the evaluation is called, the exception might be caught granularly or globally in which case evaluation skips to the next product. Logging is automatic.
• Logger::Error. over time this became the severity to log issues on internal errors in ifcopenshell
• Logger::Warning. invalid IFC data that will negatively affect output but something for the product will be visible
• Logger::Notice. invalid or unidiomatic use of IFC that is fully worked around in ifcopenshell

This is very much coming from the old days of IFC where if something doesn't show up it's assumed to be the fault of the opening application, because validity and strict usage of IFC wasn't a thing at that time. Also now with 4.3 and alignment geometry I would gravitate towards being as permissive as possible in terms of having 3d output show up (because if the user doesn't see anything visually then there also isn't an element to attach error information to except for the log, which people don't read), but log everything as warnings that you don't like to see in the model. And keep a long list of validation rules for @civilx64 to implement in the official validation service so that IFC becomes stricter. In the end, end-users compare implementations by how many models they can successfully open, not based on how compliant the implementation is to the standard.

Please point me to an example of how I can get the Dim attribute

In C++ this is unfortunately not possible yet. The Dim attribute is actually a good example of a simple attribute that will, for example in case of a Polyline, will call various functions to eventually return 2 or 3, based on the dimensionality of the points, which is HIINDEX of the coord tuple. We don't have access to this in C++, but we do in Python, because we have a code generator that turns EXPRESS function code into Python.

I haven't decided yet on the plan for this. We either have to write an interpreter for EXPRESS (because a code generator is likely not ideal, because of robustness, you can't catch every error in C++, but you can in Python) or we have to embed the Python interpreter into IfcOpenShell C++ so that from C++ you can evaluate Python to interpret the derived attributes. This is something for the future to decide, depends mostly on the community and any potential funding I guess.

Lastly, what is the preferred method for dealing with numerical tolerances?

There is an attribute Precision on the IfcGeometricRepresentationContext. This is available as conv_settings_.getValue(ConversionSettings::GV_PRECISION) in the mapping class and is adjusted for length unit (so this is the Precision in meters).

A single precision value is hardly enough though, so throughout the codebase sometimes we use arbitrary fixed constants. Especially related to angular things, but also some thicknesses because Open CASCADE does not go below 1.e-8 linear tolerance and we have to add some 'padding' around that.