I would be happier if the "collect" routine didn't do formatting (conversion to a string).

This one would take some more work and require changing other collect methods as well. I think that might be better as a separate issue/PR.

I have stumbled across a syntax that is a bit more concise. Not sure if you want to use it, though:

if getattr(node, "is_named_expression_type", bool)():

I think I'll keep the current form as it is a little easier to understand what it is doing.

I think I understand what you are doing here, but wouldn't this complain loudly about the objective for every parameter estimation problem (MSE) if the problem actually solved?

One of my main concerns with this tool is its inability to distinguish between true cases of catastrophic cancellation and benign situations, where, for example, we have a heat of adsorption term in a column which will be close to except near the breakthrough point.

Is there an easy way to test for that however? Note that this tool is only used for Cautions in the main toolbox, with the implication that these might be issues (but not guaranteeing that).

I.e., this is intended to be a simple check to find potential issues, but the user will have to look into them all further to decide if they are critical or not.

I'm not sure there's an easy way to do it without assuming a well-scaled system. In a well-scaled system, it would show up in the SVD with extremely large or extremely small singular values.

However, once you get to a high enough noise/signal ratio, you start to question about whether including the tool in report_numerical_issues is worthwhile or whether it is distracting the user from more fruitful avenues of diagnostics. I suppose we can just pull it from report_numerical_issues without a deprecation cycle if it proves to not be useful, so we can try it out and see how users find it.

@jsiirola This walker was written specifically with Constraints in mind, so I had not considered that. The check for mismatched terms would make sense for an Objective however, so this could be extended to handle them as well. However, how would the expression walker know if it was dealing with an Objective or a Constraint (the input argument is the expression, not the component)?

Don't you need to negate the values for the second argument in the relational expression before treating it like a sum?

I had forgotten to make that correction - thank you.

Don't you need special handling for Ranged (like Equality / Inequality)? I would probably define a

def _check_ranged(self, node, child_data):
    lhs_vals, lhs_mismatch, lhs_cancelling, lhs_const = self._check_equality(node, child_data[:2])
    rhs_vals, rhs_mismatch, rhs_cancelling, rhs_const = self._check_equality(node, child_data[1:])
    # Then merge the results and return

I think I've fixed this - it would be good if you could double check my logic however.

Almost all of your nonlinear handlers could be replaced by a single callback:

def _check_general_expr(self, node, child_data):
    mismatch, cancelling, const = self._perform_checks(node, child_data)
    val = node._apply_operation(list(map(self._get_value_for_sum_subexpression, child_data)))
    return [val], mismatch, cancelling, const

Thank you - I did not know I could do that.

Why do these have default values in the object when we're also giving them default values in the ConfigBlock?

Different objects - the is the visitor class which exists outside the DiagnosticsToolbox class and so can be used separately if desired. Thus, the visitor has the ability to take these as arguments and thus has default values for them.