Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce number of warnings emitted during testing #2887

Merged
merged 17 commits into from
Jan 25, 2024

Conversation

jvesely
Copy link
Collaborator

@jvesely jvesely commented Jan 25, 2024

A collection of minor fixes and updates improves the handling of warnings by updating tests to use new interfaces.
The original distribution of warnings when running 4 test jobs was 7299 emitted warnings in 211 locations:
5 DeprecationWarning
9 FutureWarning
1 MatplotlibDeprecationWarning
4 PendingDeprecationWarning
12 PNLCompilerWarning
2 PytestBenchmarkWarning
1 PytestRemovedIn8Warning
1 PytestReturnNotNoneWarning
4 RuntimeWarning
1 SyntaxWarning
171 UserWarning

Changes in this PR reduce this to 5696 emitted warnings in 191 locations:
2 FutureWarning
1 MatplotlibDeprecationWarning
4 PendingDeprecationWarning
12 PNLCompilerWarning
2 RuntimeWarning
170 UserWarning

Individual commits target different warning classes.

…es a warning

Fixes 2 instances of: UserWarning: No inputs provided in call to ...

Signed-off-by: Jan Vesely <[email protected]>
Simplify parametrization ids.

Fixes: SyntaxWarning: "is" with a literal. Did you mean "=="?
Signed-off-by: Jan Vesely <[email protected]>
Return both result and num_executions_before_finished.

Fixes: PytestBenchmarkWarning: Benchmark fixture was not used at all in this test!
Signed-off-by: Jan Vesely <[email protected]>
Makes the test benchmark agnostic.
Remove explicit benchmark re-run from the test.

Fixes: PytestBenchmarkWarning: Benchmark fixture was not used at all in this test!
Signed-off-by: Jan Vesely <[email protected]>
Having multiple parameter ports for parameters of the same name is now an
error.
There is no difference in emitted warnings with or without this filter.

Signed-off-by: Jan Vesely <[email protected]>
Tests should not return anything.
Fixes: PytestReturnNotNoneWarning: Expected None, but tests/composition/test_composition.py::TestNestedCompositions::test_invalid_projection_deletion_when_nesting_comps returned (Composition ocomp), which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?

Signed-off-by: Jan Vesely <[email protected]>
The construct has been deprecated in pytest warns about it:
PytestRemovedIn8Warning: Passing None has been deprecated.
  See https://docs.pytest.org/en/latest/how-to/capture-warnings.html#additional-use-cases-of-warnings-in-tests for alternatives in common use cases.

Address three different situations in tests in a way that does not hide
unrelated warnings.
 * test_composition.py::*_subset_duplicate_warnings:
   capture all warnings and search them for the expected message if
   verbosity == True
   assert that there are no warnings if verbosity == False

 * test_projection_specifications.py::test_no_warning_when_matrix_specified:
   Use filterwarnings mark to change the undesired warning into an error

 * test_control.py::test_model_based_num_estimates
   Use null context or pytest.warns context based on whether warning is
   expected

Signed-off-by: Jan Vesely <[email protected]>
Square brackets '[' denote a match set and need to be escaped.
Fixes: FutureWarning: Possible nested set at position ...

Signed-off-by: Jan Vesely <[email protected]>
Convert tested value to np.array and check its size.

Fixes:
  DeprecationWarning: The truth value of an empty array is ambiguous.  ...
  DeprecationWarning: elementwise comparison failed; ...
Signed-off-by: Jan Vesely <[email protected]>
The expression replaces calculated np.log values with 0
if both v1 and v2 values at a given index are 0.
Skip the computation of np.log(v2) in this case to avoid invalid values.

Fixes:
  RuntimeWarning: divide by zero encountered in log
  RuntimeWarning: invalid value encountered in multiply

Signed-off-by: Jan Vesely <[email protected]>
Otherwise they might end up compared as containers.

Fixes:
   FutureWarning: elementwise comparison failed; ...
Signed-off-by: Jan Vesely <[email protected]>
Fixes 3 instances of:
  FutureWarning: Setting parameter values directly using dot notation may be removed in a future release.

Signed-off-by: Jan Vesely <[email protected]>
…rix() instead of mod_matrix

The latter is deprecated.

Signed-off-by: Jan Vesely <[email protected]>
@jvesely jvesely added the tests Test changes label Jan 25, 2024
Copy link

This PR causes the following changes to the html docs (ubuntu-latest-3.11-x64):

diff -r docs-base/TransferMechanism.html docs-head/TransferMechanism.html
566c566
< <div class="highlight-default notranslate"><div class="highlight"><pre><span></span><span class="gp">&gt;&gt;&gt; </span><span class="n">my_linear_tm</span><span class="o">.</span><span class="n">noise</span> <span class="o">=</span> <span class="p">[</span><span class="mf">1.0</span><span class="p">,</span><span class="mf">1.2</span><span class="p">,</span><span class="mf">.9</span><span class="p">]</span>
---
> <div class="highlight-default notranslate"><div class="highlight"><pre><span></span><span class="gp">&gt;&gt;&gt; </span><span class="n">my_linear_tm</span><span class="o">.</span><span class="n">noise</span><span class="o">.</span><span class="n">base</span> <span class="o">=</span> <span class="p">[</span><span class="mf">1.0</span><span class="p">,</span><span class="mf">1.2</span><span class="p">,</span><span class="mf">.9</span><span class="p">]</span>

...

See CI logs for the full diff.

@jvesely
Copy link
Collaborator Author

jvesely commented Jan 25, 2024

Remaining warnings:

  • 2 FutureWarning
    Both are issued because of the example in the ParameterPort documentation. I didn't change these to use param.modulated because the documentation explicitly talks about the fact that mod_* is available on Mechanism even for parameters of the owned function

  • 1 MatplotlibDeprecationWarning
    This is also blocking matplotlib version bump (that turns this warning into an error), see models/MontagueDayanSejnowski: Use versioned seaborn style #2808, We can't switch to the new name python3.7 is limited to matplotlib 3.5.3, which doesn't have any of the new names.

  • 4 PendingDeprecationWarning
    All 4 are warnings about using numpy.matrix. These are responsible for the majority of the remaining warning instances (3631/5696). There was an old effort to remove np.matrix usage, but we can also just silence them until numpy removes support for 'matrix' entirely.

  • 12 PNLCompilerWarning
    All of them are shape mismatch warnings.

  • 2 RuntimeWarning
    One is in the already updated CombinationFunction. The one case that remains is in cross_entropy_loss test where only the second vector is 0 resulting in np.log(0).
    The second warning is trying to normalize 0 matrix in transferfunctions.py:3960

  • 170 UserWarning
    Various PNL issued warnings. Most of them should be possible to silence by updating tests to fix the issues they warn about or adding a warning check if a PNL warning is expected.

Copy link

This PR causes the following changes to the html docs (ubuntu-latest-3.11-x64):

diff -r docs-base/ModulatoryProjection.html docs-head/ModulatoryProjection.html
272c272
< <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="ParameterPort.html#psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents" title="psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
---
> <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="Mechanism.html#id15" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
diff -r docs-base/Port.html docs-head/Port.html
593c593
< <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="#psyneulink.core.components.ports.port.Port_Base.path_afferents" title="psyneulink.core.components.ports.port.Port_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
---
> <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="Mechanism.html#id14" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
diff -r docs-base/TransferFunctions.html docs-head/TransferFunctions.html
2263c2263
< <p><a class="reference internal" href="OptimizationFunctions.html#psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds" title="psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to
---
> <p><a class="reference internal" href="#psyneulink.core.components.functions.transferfunctions.Exponential.bounds" title="psyneulink.core.components.functions.transferfunctions.Exponential.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to
diff -r docs-base/TransferMechanism.html docs-head/TransferMechanism.html
566c566
< <div class="highlight-default notranslate"><div class="highlight"><pre><span></span><span class="gp">&gt;&gt;&gt; </span><span class="n">my_linear_tm</span><span class="o">.</span><span class="n">noise</span> <span class="o">=</span> <span class="p">[</span><span class="mf">1.0</span><span class="p">,</span><span class="mf">1.2</span><span class="p">,</span><span class="mf">.9</span><span class="p">]</span>
---
> <div class="highlight-default notranslate"><div class="highlight"><pre><span></span><span class="gp">&gt;&gt;&gt; </span><span class="n">my_linear_tm</span><span class="o">.</span><span class="n">noise</span><span class="o">.</span><span class="n">base</span> <span class="o">=</span> <span class="p">[</span><span class="mf">1.0</span><span class="p">,</span><span class="mf">1.2</span><span class="p">,</span><span class="mf">.9</span><span class="p">]</span>

...

See CI logs for the full diff.

@jvesely jvesely merged commit 26f6b64 into PrincetonUniversity:devel Jan 25, 2024
32 checks passed
@jvesely jvesely deleted the warn branch January 25, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants