Register NLparameters and pretty print them

[davidanthoff]

Fixes #2509.

I have to admit I'm not sure this makes sense or whether the implementation is a good idea. But it does start to print the parameter name in the show output rather than writing parameter_1 etc :) But in any case, someone with a better understanding of the code base needs to weigh in and say whether this could potentially work or not.

[odow]

One issue is that this is actually a breaking change because this will now error (where previously it worked):

@variable(model, x)
@NLparameter(model, x == 1)

The code to look up a name isn't great (this only works for single parameters) but it is an improvement. It also only prints in subexpressions, not pure parameters

JuMP.jl/src/print.jl

Lines 752 to 758 in 442aee8

	function function_string(::Type{<:PrintMode}, p::NonlinearExpression)
	return "Reference to nonlinear expression #$(p.index)"
	end
	function function_string(::Type{<:PrintMode}, p::NonlinearParameter)
	return "Reference to nonlinear parameter #$(p.index)"
	end

Tests are failing here:

JuMP.jl/test/print.jl

Lines 250 to 255 in 442aee8

	"(subexpression[1] - parameter[1]) - 0.0 $le 0",
	)
	io_test(
	IJuliaMode,
	constr,
	"(subexpression_{1} - parameter_{1}) - 0.0 \\leq 0",

If you have code that would benefit from registering parameters, you can just go

model[:p] = @NLparameter(model, p == 1)

[davidanthoff]

I'm really just interested in the pretty printing at the moment. So maybe the way to change this PR is to only merge the pretty printing side, which would be non breaking, and then I can do the registration step manually on my end?

Having said that, it might make sense to bring in the breaking change of not allowing variables and parameters to have the same name in some new major version? That does seem like a sensible default? But obviously no rush on that.

For now I'll change this PR to only do the pretty printing, and I'll also try to make that a bit more robust.

[codecov]

Codecov Report

Merging #2516 (c94c6b5) into master (442aee8) will decrease coverage by 4.36%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2516      +/-   ##
==========================================
- Coverage   92.42%   88.05%   -4.37%     
==========================================
  Files          43       42       -1     
  Lines        4688     3802     -886     
==========================================
- Hits         4333     3348     -985     
- Misses        355      454      +99     

Impacted Files	Coverage Δ
src/nlp.jl	90.50% <100.00%> (-1.83%)	⬇️
src/print.jl	85.28% <100.00%> (-5.74%)	⬇️
src/shapes.jl	33.33% <0.00%> (-66.67%)	⬇️
src/utils.jl	33.33% <0.00%> (-23.81%)	⬇️
src/Containers/Containers.jl	71.42% <0.00%> (-17.47%)	⬇️
src/Containers/SparseAxisArray.jl	67.16% <0.00%> (-17.05%)	⬇️
src/Containers/container.jl	83.33% <0.00%> (-11.41%)	⬇️
src/sets.jl	78.94% <0.00%> (-11.06%)	⬇️
src/Containers/vectorized_product_iterator.jl	35.71% <0.00%> (-10.96%)	⬇️
src/aff_expr.jl	85.00% <0.00%> (-10.11%)	⬇️
... and 33 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 442aee8...f10c416. Read the comment docs.

[davidanthoff]

Alright, so this PR now has the registration of parameters removed, and all it does is use the name of a parameter in printing, if that parameter was manually registered with the model.

[odow]

Yeah, we could definitely do better than this, but I think it will take a much bigger NLP redesign.