Removing enum ConstraintType

It's time to think about removing the enums in wolf.h. This issue concerns the ConstraintType enum.

The idea is to be able to completely avoid these global enums. This first post just exposes the current situation.

This enum is used in switch / case statements in three places:

• In SolverManager:

ceres::CostFunction* SolverManager::createCostFunction(ConstraintBasePtr _corrPtr)
{
	switch (_corrPtr->getConstraintType())
	{
		case CTR_GPS_FIX_2D:
		{
			ConstraintGPS2D* specific_ptr = (ConstraintGPS2D*)(_corrPtr);
			return new ceres::AutoDiffCostFunction<ConstraintGPS2D,
													specific_ptr->measurementSize,
													specific_ptr->block0Size,
													specific_ptr->block1Size,
													specific_ptr->block2Size,
													specific_ptr->block3Size,
													specific_ptr->block4Size,
													specific_ptr->block5Size,
													specific_ptr->block6Size,
													specific_ptr->block7Size,
													specific_ptr->block8Size,
													specific_ptr->block9Size>(specific_ptr);
			break;
		}
		case CTR_ODOM_2D_COMPLEX_ANGLE:
		{
            [...]
        }
    }

• In qr_solver.h :

        CostFunctionBasePtr createCostFunction(ConstraintBasePtr _corrPtr)
        {
            //std::cout << "adding ctr " << _corrPtr->nodeId() << std::endl;
            //_corrPtr->print();

            switch (_corrPtr->getTypeId())
            {
                case CTR_GPS_FIX_2D:
                {
                    ConstraintGPS2D* specific_ptr = (ConstraintGPS2D*)(_corrPtr);
                    return (CostFunctionBasePtr)(new CostFunctionSparse<ConstraintGPS2D, specific_ptr->measurementSize,
                            specific_ptr->block0Size, specific_ptr->block1Size, specific_ptr->block2Size,
                            specific_ptr->block3Size, specific_ptr->block4Size, specific_ptr->block5Size,
                            specific_ptr->block6Size, specific_ptr->block7Size, specific_ptr->block8Size,
                            specific_ptr->block9Size>(specific_ptr));
                    break;
                }
                case CTR_ODOM_2D:
                {
                    [...]
                }
            }
        }

• In create_autodiff_cost_function.cpp

ceres::CostFunction* createAutoDiffCostFunction(ConstraintBasePtr _ctr_ptr, bool _use_wolf_autodiff)
{
    switch (_ctr_ptr->getTypeId())
    {
        case CTR_GPS_FIX_2D:
            if (_use_wolf_autodiff)
                return createAutoDiffCostFunctionWrapper<ConstraintGPS2D>(_ctr_ptr);
            else
                return createAutoDiffCostFunctionCeres<ConstraintGPS2D>(_ctr_ptr);

        case CTR_FIX:
        [...]
    }
}

Mentioned in issue #98 (closed)

@joanvallve could we give an end to this issue? I think almost, no? What is missing?

In eclipse, I have warnings of the type Member 'jets_0_' was not initialized in this constructor constraint_autodiff.h /wolf/src line 403 Code Analysis Problem

We also may want to delete the obsolete files remaining in /ceres_wrapper...

If we want to use ceres numeric differentiation, there is still an anoing switch like those.. The obsolete files remaining in /ceres_wrapper are deleted in the jet_autodiff branch.

And the numeric also has the issue with the switch?

By the way, the issue demands for removing the enum. This implies getting rid of all the uses for this ConstraintType. In any case, what I did for all other wolf classes is to use a string type, just in case we want to identify them for any reason. A string is not centralized so each derived class can define its own type with an string.

Yes, for using ceres numeric differentiation, we still need to cast... but we can find out a solution again using CRTP. Yes, an string is ok.

Ceres numeric differentiation is quite good and it's not easily copyable as autodiff. Also, if we want to keep ceres dependency out of wolf library, I don't see a way to do it without any switch. So, if we want to be able to do numeric differentiation using ceres, there will be a switch in create_numeric_diff_cost_function.h, so the string is needed.

But we can remove the ENUMs for sure

assigned to @joanvallve

done

closed