Add Pluralization of @NLparameters (#2593)

[Pramodh-G]

Fixes Issue #2593 .
Please suggest ways in which this PR can be improved :)

[codecov]

Codecov Report

Merging #2619 (81f6d23) into master (972b97c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2619   +/-   ##
=======================================
  Coverage   93.50%   93.50%           
=======================================
  Files          44       44           
  Lines        5544     5544           
=======================================
  Hits         5184     5184           
  Misses        360      360           

Impacted Files	Coverage Δ
src/macros.jl	92.96% <ø> (ø)

---

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 972b97c...81f6d23. Read the comment docs.

[patrickocal]

May I suggest that you make clear the syntax for begin ... end blocks:

"Distinct parameters need to be placed on seperate lines without base_names as in the following example."

This is because other objects (such as variables and constraints) can be assigned names.

[Pramodh-G]

Sure, I'll update it.

[patrickocal]

What was meant is that somewhere it should be clear that, unlike variables and constraints, parameters cannot be given a ```name::String```.

…

On Tue, Jun 1, 2021 at 6:49 AM Oscar Dowson ***@***.***> wrote: ***@***.**** approved this pull request. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2619 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKZONACETRL4RTAV7J334UDTQPY6DANCNFSM452FJUIA> .

[odow]

What was meant is that somewhere it should be clear that, unlike variables and constraints, parameters cannot be given a name::String.
…

This is covered in the docstring of @NLparameter: https://jump.dev/JuMP.jl/v0.21.8/reference/nlp/#ref_nl_parameters. Adding extra arguments is a separate issue.

[patrickocal]

We wouldn't want any redundancy now would we :)

I think you're assuming that someone will first learn the singular and then the plural. But things don't always work that way in practice.

The fact that NLparameters is a pluralisation of NLparameter and constructed via a macro is incidental to the way JuMP was built. It has nothing to do with the perspective of the user.

[odow]

We link to the @NLparameter docstring and explain it follows the same syntax. Moreover, the general rule for docstrings is to explain the things that can be done, rather than what can't be done.

Error messages should tell what people did wrong (this isn't the greatest example because it actually looks like a bug!:

julia> @NLparameter(model, p == 1, base_name = "x")
ERROR: LoadError: At REPL[7]:1: `@NLparameter(model, p == 1)`: Too many arguments.
Stacktrace:
 [1] error(::String, ::String)
   @ Base ./error.jl:42
 [2] _macro_error(macroname::Symbol, args::Tuple{Symbol, Expr}, source::LineNumberNode, str::String)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:1365
 [3] (::JuMP.var"#_error#122"{LineNumberNode, Symbol, Expr})(str::String)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:2114
 [4] var"@NLparameter"(__source__::LineNumberNode, __module__::Module, m::Any, ex::Any, extra::Vararg{Any, N} where N)
   @ JuMP ~/.julia/dev/JuMP/src/macros.jl:2118
in expression starting at REPL[7]:1

Edit: I opened an issue: #2620