Hello.
Le lun. 11 avr. 2022 à 07:36, Avijit Basak <avijit.ba...@gmail.com> a écrit :
>
> [...]
>
> >I can't seem to find how the logger is configured. Currently, all
> >"INFO" messages are logged to the "standard error" console; one
> >should be able to e.g. redirect output to a file, or set the log level.
> -- There is a dependency for "slf4j-simple" in the "examples-ga" module
> which would print all log messages in the console.
Currently; it prints everything on "stderr" (so that simple usage of
the UNIX shell's "pipe" syntax is not possible).
> There was a discussion
> earlier on this and the decision was to use console for example modules for
> log messages.
I recall that we want a separation between the logger API (library
dependency) and logger implementation ("examples" dependency).
Is it a feature of "slf4j-simple" to only print to "stderr" or can it
be configured?
There is no issue on the "examples" module to depend on a
full-featured logger (primary candidate would be "Log4j2").
> Library users would add the implementation of slf4j according
> to their need and provide required configurations to control the log levels
> and messages.
We (developers) also want minimal flexibility for testing purposes...
Also, IMO "examples" codes should make a difference
between "logging" (optional information about internals)
and "output" (mandatory result).
For example, the TSP application result (itinerary and travel
distance) could be saved in a file whose name is given as
argument to a mandatory "--output" command-line option, as
in module "commons-math-examples/examples-sofm/tsp".
> >
> >There is still a mix between library code and application code (but
> >this is to be discussed in MATH-1643.
> -- Kindly update the JIRA with your suggestions.
Sure, we'll come to that. But I think we should first converge to
a codebase that can be merged to "master" as a replacement of
the "o.a.c.math4.genetics" package (to be removed).
Do you confirm that all the unit tests in that package have an
equivalent in the new "commons-math-ga" module?
> >From browsing the library code, I'm tempted to believe that the
> >dependency towards a logging framework is not necessary (or
> >underused).
> -- Do you mean dependency on the slf4j?
Yes.
> I think that such a feature could be left to the application
> >layer (per the "ConvergenceListener" registry).
> -- ConvergenceListener is only to log the nature of convergence,
> not internal details of all operators. Currently we have a very low number
> of log messages which can be increased later.
"Increased later" for what purpose?
Logging can be very useful (e.g. for debugging); however once
this new implementation is trusted to behave correctly, I don't see
that additional logging statements inside the library will be very
useful; they would rather be needed by the application in order to
e.g. display the content of the current "Population". The "data" (to
be eventually displayed) would be provided by the library through
regular accessors.
Another case in point is that the library cannot know what display
format is suitable for the application (e.g. a GUI).
If we really need (not clear as of the current codebase) to provide
runtime access to other parts of the library (operators, etc.), why
not generalise the "observer" pattern?
>
> >Likewise, the "PopulationStatisticsLogger" is not general enough to
> >be worth being part of the library.
> -- I would love to keep this simple implementation of convergence listener.
> This can provide a very basic overview on the nature of convergence.
As a user, you'll be able to provide the feature to your application
with less than 10 lines of code (by registering a "listener").
Everyone else might want a slightly different output (contents and/or
format) than what this class generates.
> >A few (nit-pick) remarks about code style in general.
> >Javadoc is incomplete: All methods must be documented.
> >Please avoid redundant links like e.g.
> >---CUT---
> > /**
> > * @param crossoverPolicy The {@link CrossoverPolicy}
> > * @param mutationPolicy The {@link MutationPolicy}
> > * @param selectionPolicy The {@link SelectionPolicy}
> > * @param convergenceListeners An optional collection of
> > * {@link ConvergenceListener} with
> >variable arity
> > */
> > @SafeVarargs
> > protected AbstractGeneticAlgorithm(final CrossoverPolicy<P>
> crossoverPolicy,
> > final MutationPolicy<P> mutationPolicy,
> > final SelectionPolicy<P> selectionPolicy,
> > ConvergenceListener<P>... convergenceListeners) {
> > this.crossoverPolicy = crossoverPolicy;
> > this.mutationPolicy = mutationPolicy;
> > this.selectionPolicy = selectionPolicy;
> > updateListenerRigistry(convergenceListeners);
> > }
> >---CUT---
> >Readers of the HTML-generated doc can already click on the various
> >arguments within the signature; so there is no need to add visual noise
> >in the source code just to be able to click from within the Javadoc part
> >just above that signature.
> >The Javadoc block above should be
> >---CUT---
> > /**
> > * @param crossoverPolicy Crossover policy.
> > * @param mutationPolicy Mutation policy.
> > * @param selectionPolicy Selection policy.
> > * @param convergenceListeners Collection of user-defined listeners.
> > */
> >---CUT---
> >[Note the absence of "The" and the presence of a final "period".]
> >
> -- Are we following this standard in the commons-math project?
Yes.
Not everybody follows strict rules, and certainly not all "Commons"
components have the same rules (unfortunately), but since it was
decided that this module belongs in "Commons Math", I'd like to
minimize inconsistent coding styles (converging on what is used in
non-"legacy" source files).
> This would
> initiate changes in almost all classes.
Do you mean "all classes" in the GA module?
Then so be it. ;-)
>
> >A blank line is welcome to separate ideas ("logical" blocks of code)
> >However, there should not be an empty line after a closing brace if
> >it is followed by another closing brace.
> >Also, in all recent codes, there is no blank line between the instance
> >fields; the (mandatory) Javadoc is enough to logically (and visually)
> >separate the fields.
> -- I have rectified this.
Thanks!
> [...]
>
> >* Are annotations (@SafeVarargs, ...) necessary? Please document.
> -- This annotation is necessary for any parameterized vararg. This is also
> used in legacy classes like o.a.c.m.l.a.i.FieldHermiteInterpolator and
> o.a.c.m.l.o.n.RungeKuttaFieldStepInterpolator.
Hmm, another point to discuss later.
>
> >In "AdaptiveGeneticAlgorithm":
> >* There should be a single constructor (same remark as above).
> -- Removed the constructor with default argument.
>
> >* Why the use of reflection ("isAssignableFrom")?
> -- Replaced it by instanceof.
Marginally better ;-) it still does not say why the statistics is disabled
depending on the operator type...
Regards,
Gilles
>> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
