fit generates invalid sig_hash for ArrayXb terminal nodes

[gAldeia]

fit generates invalid sig_hash for ArrayXb terminal nodes

[gAldeia]

After talking with @lacava, we found that boolean values were missing in signature.h file:

brush/src/program/signatures.h

Lines 196 to 201 in 546c2a7

	struct Signatures<N, enable_if_t<is_in_v<N, NodeType::Constant, NodeType::Terminal>>>{
	using type = std::tuple<
	Signature<ArrayXf()>,
	Signature<ArrayXi()>
	>;
	};

After adding it, I had to make some changes so:

1. Terminal nodes with boolean features cannot be weighted;
2. get_weights knows about 1 and behave accordingly.

For 1, I created a new function to check if a node's ret_type is one of the unweightable types:

brush/src/program/node.h

Lines 43 to 62 in 25c8674

	template <DataType... T>
	inline auto Isnt(DataType dt) -> bool { return !((dt == T) || ...); }
	template<DataType DT>
	inline auto IsWeighable() noexcept -> bool {
	return Isnt<DataType::ArrayB,
	DataType::MatrixB,
	DataType::TimeSeriesB,
	DataType::ArrayBJet,
	DataType::MatrixBJet
	>(DT);
	}
	inline auto IsWeighable(DataType dt) noexcept -> bool {
	return Isnt<DataType::ArrayB,
	DataType::MatrixB,
	DataType::TimeSeriesB,
	DataType::ArrayBJet,
	DataType::MatrixBJet
	>(dt);
	}

and started using it in the Node's constructor.

I also changed how we manipulate the is_weighted: now we should use a getter and setter (as we already have for other attributes). The setter performs a check before enabling a node to be weightable:

brush/src/program/node.h

Lines 248 to 260 in 25c8674

	inline bool get_is_weighted() const {return this->is_weighted;};
	inline void set_is_weighted(bool is_weighted){
	// cant change the weight of a boolean terminal
	if (IsWeighable(this->ret_type))
	this->is_weighted = is_weighted;
	};
	private:
	/// @brief feature name for terminals or splitting nodes
	string feature;
	};

For 2, I've implemented a different function to get the weights of a boolean terminal (and changed the original one to require that the value is not boolean). The comment inside this function explains why it exists:

brush/src/program/operator.h

Lines 10 to 58 in 25c8674

	namespace util{
	////////////////////////////////////////////////////////////////////////////////
	/// @brief get weight
	/// @tparam T return type
	/// @tparam Scalar scalar type of return type
	/// @param tn tree node
	/// @param weights option pointer to a weight array, used in place of node weight
	/// @return
	template<typename T, typename Scalar, typename W>
	requires (!is_one_of_v<Scalar,bool, bJet>)
	Scalar get_weight(const TreeNode& tn, const W** weights=nullptr)
	{
	Scalar w;
	// Prediction case: weight is stored in the node data.
	if (weights == nullptr)
	{
	w = Scalar(tn.data.W);
	}
	else
	{
	// NLS case 1: floating point weight is stored in weights
	if constexpr (is_same_v<Scalar, W>)
	w = **weights;
	// NLS case 2: a Jet/Dual weight is stored in weights, but this constant is a
	// integer type. We need to do some casting
	else if constexpr (is_same_v<Scalar, iJet> && is_same_v<W, fJet>) {
	using WScalar = typename Scalar::Scalar;
	WScalar tmp = WScalar((**weights).a);
	w = Scalar(tmp);
	}
	// NLS case 3: a Jet/Dual weight is stored in weights, matching Scalar type
	else
	w = Scalar(**weights);
	*weights = *weights+1;
	}
	return w;
	};
	template<typename T, typename Scalar, typename W>
	requires (is_one_of_v<Scalar,bool, bJet>)
	Scalar get_weight(const TreeNode& tn, const W** weights=nullptr)
	{
	// we cannot weight a boolean feature. Nevertheless, we need to provide
	// an implementation for get_weight behavior, so the metaprogramming
	// doesn't fail to get a matching signature.
	return Scalar(true);
	};
	}

These changes resulted in a new brush version that successfully passes on all tests, including the most recent test_data.cpp that I wrote specifically to fail due to the bug reported in this issue.

I think we can close this issue.

[gAldeia]

This was solved in the PR #43.