Feasibility checker with function

[blegat]

In the primal_feasibility_report just introduced by #2466, we expect the primal value to be given by a Dict. In the future, we might rely on MOIU.eval_variables to evaluate the function which requires a function mapping the variables to their values as arguments so we will have to create a lambda from the Dict. In view of this, we could also accept a function as argument to specify the value of the variables.
To allow the syntax:

primal_feasibility_report(model, tol) do var_ref
    ...
end

we should have the point as a first argument while it is currently the second argument.

[odow]

We made it an AbstractDict and not a Dict so you can go:

struct FunctionalDict{F} <: AbstractDict
    f::F
end
Base.getindex(f::FunctionalDict, key::VariableRef) = f.f(key)

FunctionalDict(x -> value(x))

Do we have a concrete use-case for this? I'm not in favor of adding complexity and multiple ways to do something before we really need to.

[blegat]

Currently, we convert all function to JuMP before evaluating them. When we'll start looking at performance, we'll notice that we should directly evaluate them at the MOI level hence use:
https://github.com/jump-dev/MathOptInterface.jl/blob/master/src/Utilities/functions.jl#L6-L13
so passing a function is more efficient.
There are indeed two use cases. If the user has a costly way to get the value of the variables then he should first create a dict to avoid computing it twice. Otherwise, it's easier and more efficient to just pass a function.

[odow]

We already evaluate the functions using a function:

JuMP.jl/src/feasibility_checker.jl

Lines 60 to 69 in e4320ea

	function point_f(x::VariableRef)
	fx = get(point, x, missing)
	if ismissing(fx) && !skip_missing
	error(
	"point does not contain a value for variable $x. Provide a " *
	"value, or pass `skip_missing = true`.",
	)
	end
	return fx
	end

JuMP.jl/src/feasibility_checker.jl

Line 108 in e4320ea

d = _distance_to_set(value.(obj.func, point_f), obj.set)

[odow]

To summarize the developer call:

• primal_feasibility_report(model::Model, point::AbstractDict) should stay.
• At some point in the future, we can add a non-breaking primal_feasibility_report(point::Function model::Model)
• Even if we add the functional form is added, we should not add primal_feasibility_report(point::AbstractDict, model::Model).
• Before adding the functional form, we should wait until there is evidence that the functional form is more convenient than the dictionary.

I would then propose that we close this issue for now and re-visit it in the future.

[blegat]

Thinking more about this. I agree that if the user pass a function, he might expect that we memoize its results but we don't so that's an issue. In fact, this also applies to MOI functions. Even ScalarAffineFunction can have several terms with the same variable.
If the user has some custom way. In view of this, we should probably change JuMP.value so that it expects a dictionary and MOI.eval_variable so that it takes a dictionary as second argument.
We should check that we agree on a long term plan to have a consistent interface between primal_feasibility_checker, value and eval_variable before going forward

[odow]

The functional form of value is useful:

JuMP.jl/src/callbacks.jl

Lines 64 to 66 in 2b08a39

	function callback_value(cb_data, expr::Union{GenericAffExpr,GenericQuadExpr})
	return value(expr, v -> callback_value(cb_data, v))
	end

JuMP.jl/src/aff_expr.jl

Lines 488 to 490 in 2b08a39

	function value(a::GenericAffExpr; result::Int = 1)
	return value(a, (x) -> value(x; result = result))
	end

The value functions are different, because they often apply to a very small subset of the model. (You don't want have to build a dictionary of every variable value just to call callback_value.) primal_feasibility_report applies to the whole model.

So I don't think we need a consistent interface between primal_feasibility_checker and value. I think the design, as we have it, makes senses in both cases.