[WIP] Enable full tests for Formal Integral #1499

aribalam · 2021-03-19T12:12:40Z

Description

Fixes #1465.
The PR reintroduces tests for Formal Integrals which was apparently disabled earlier due to a memory problem.

Note: The PR for updating reference data has been merged in the tardis-refdata repository

Motivation and Context

How Has This Been Tested?

Testing pipeline
Reference Data Comparison following these instructions
Other (please describe)

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
None of the above (please describe)

Checklist:

My code follows the code style of this project
My change requires a change to the documentation
I have updated the documentation accordingly
I have built the documentation on my fork following these instructions
I have assigned and requested two reviewers for this pull request

andrewfullard · 2021-03-19T15:10:40Z

I recommend running the local test pipeline to determine the cause of test failures. Glancing at the logs it appears that data required for the test are not available in the test HDF file (a storage of the test data).

aribalam · 2021-03-20T19:06:19Z

@andrewfullard I think the problem lies here in the comment.
The test HDF file is not available as it is not getting generated due to the error mentioned here.
I can commit the changes as mentioned in the first link to my comment above and check if its working.

andrewfullard · 2021-03-22T15:14:23Z

Please do. Thanks for the work on this

aribalam · 2021-03-22T16:45:01Z

The tests are still failing due to the fact that the unit_test_data.h5 that the azure pipeline is referring to has not been updated with the current tardis-refdata repository. This is because these tests are passing locally on my machine.
I came across an azure pipeline update-refdata.yml (link here) that seems to achieve the said task.
Maybe @epassaro can help on this?

epassaro · 2021-03-22T18:24:20Z

As you can see in the Azure repo history the commit hash is exactly the same as the latest tardis-refdata.

EDIT 1: I tested your branch on my computer and got 12 tests failed (same as Azure). I assume you didn't use the reference data flag when calling pytest.

EDIT 2: @andrewfullard My guess is these keys got removed in Nov 2021 (see here) after marking those tests as skip. Since we're creating a new HDF5 file every time we generate reference data skipped tests would not be included. Is that clear enough?

aribalam · 2021-03-23T20:50:09Z

@epassaro That clears everything.
One more thing. When the tests are run using python setup.py test --args="--tardis-refdata=<refdata dir>". The result comes out to be different than what we get here.
The test results are as follows -

============================================================================ short test summary info =============================================================================
FAILED tardis/montecarlo/tests/test_packet_source.py::test_bb_packet_sampling - assert False
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[nu_bar_estimator] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[j_estimator] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[t_radiative] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[dilution_factor] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[output_nu] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_estimates[output_energy] - AssertionError: Series are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_state_iterations[iterations_w] - AssertionError: DataFrame.iloc[:, 0] (column name="0") are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_state_iterations[iterations_t_rad] - AssertionError: DataFrame.iloc[:, 0] (column name="0") are different
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_state_iterations[iterations_electron_densities] - AssertionError: DataFrame.iloc[:, 0] (column name="0") are dif...
FAILED tardis/simulation/tests/test_simulation.py::test_plasma_state_iterations[iterations_t_inner] - AssertionError: Series are different
FAILED tardis/tests/test_tardis_full.py::TestRunnerSimple::test_j_blue_estimators - AssertionError: 
FAILED tardis/tests/test_tardis_full.py::TestRunnerSimple::test_spectrum - AssertionError: 
FAILED tardis/tests/test_tardis_full.py::TestRunnerSimple::test_virtual_spectrum - AssertionError: 
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_j_blue_estimators[-1-downbranch] - KeyError: 'No object named test_runner_simple_i...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum[-1-downbranch] - KeyError: 'No object named test_runner_simple_integral_d...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum_integrated[-1-downbranch] - KeyError: 'No object named test_runner_simple...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_j_blue_estimators[30-downbranch] - KeyError: 'No object named test_runner_simple_i...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum[30-downbranch] - KeyError: 'No object named test_runner_simple_integral_d...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum_integrated[30-downbranch] - KeyError: 'No object named test_runner_simple...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_j_blue_estimators[30-macroatom] - KeyError: 'No object named test_runner_simple_in...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum[30-macroatom] - KeyError: 'No object named test_runner_simple_integral_ma...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum_integrated[30-macroatom] - KeyError: 'No object named test_runner_simple_...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_j_blue_estimators[-1-macroatom] - KeyError: 'No object named test_runner_simple_in...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum[-1-macroatom] - KeyError: 'No object named test_runner_simple_integral_ma...
FAILED tardis/tests/test_tardis_full_formal_integral.py::TestRunnerSimpleFormalInegral::test_spectrum_integrated[-1-macroatom] - KeyError: 'No object named test_runner_simple_...
====================================================== 26 failed, 1177 passed, 207 skipped, 6 xpassed in 206.84s (0:03:26) =======================================================

I was expecting the results to be the same as the one we get here using pytest tardis --tardis-refdata=.... Am I missing something?

epassaro · 2021-03-23T20:54:04Z

I think using pytest tardis ... requires installing the package with python setup.py develop but I'm not sure.

andrewfullard · 2021-03-24T13:32:55Z

I believe python setup.py test is deprecated.

aribalam · 2021-03-26T18:51:21Z

@epassaro @andrewfullard I have updated the reference data and opened a PR in the refdata repository (link here). Let me know if anything else is required!

andrewfullard · 2021-04-09T13:33:14Z

For some reason, a manual re-run of the tests is not working (Azure seems to have lost the test run) so if you make a commit to this branch it should trigger them again

codecov · 2021-04-09T19:17:11Z

Codecov Report

Merging #1499 (a2ba449) into master (cf8900f) will increase coverage by 0.95%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1499      +/-   ##
==========================================
+ Coverage   68.12%   69.08%   +0.95%     
==========================================
  Files          68       68              
  Lines        6040     6100      +60     
==========================================
+ Hits         4115     4214      +99     
+ Misses       1925     1886      -39

Impacted Files	Coverage Δ
tardis/tardis/visualization/tools/sdec_plot.py	`14.17% <0.00%> (-2.12%)`	⬇️
...is/tardis/plasma/properties/continuum_processes.py	`38.48% <0.00%> (-0.73%)`	⬇️
tardis/tardis/plasma/properties/ion_population.py	`86.13% <0.00%> (-0.14%)`	⬇️
tardis/tardis/base.py	`5.55% <0.00%> (ø)`
tardis/tardis/simulation/base.py	`85.71% <0.00%> (ø)`
tardis/tardis/io/atom_data/base.py	`90.80% <0.00%> (ø)`
...s/tardis/plasma/properties/property_collections.py	`100.00% <0.00%> (ø)`
...rdis/plasma/properties/transition_probabilities.py	`32.46% <0.00%> (ø)`
...rdis/tardis/montecarlo/montecarlo_numba/vpacket.py	`17.64% <0.00%> (+0.17%)`	⬆️
...dis/montecarlo/montecarlo_numba/numba_interface.py	`50.00% <0.00%> (+1.51%)`	⬆️
... and 2 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 cf8900f...a2ba449. Read the comment docs.

* remove pytest mark for skipping the tests * set overwrite option to True * empty commit to rerun tests

remove pytest mark for skipping the tests

f942315

andrewfullard requested review from andrewfullard and Rodot- March 19, 2021 14:27

andrewfullard added the testing 🚦 label Mar 19, 2021

andrewfullard marked this pull request as draft March 19, 2021 14:30

andrewfullard added the work in progress 🚧 label Mar 19, 2021

set overwrite option to True

d995a14

aribalam force-pushed the formal_integral_tests branch from 6f1820a to d995a14 Compare March 22, 2021 14:32

andrewfullard mentioned this pull request Mar 24, 2021

Need formal integral test for C code to compare new Numba code to #1448

Closed

aribalam mentioned this pull request Mar 26, 2021

Updated test data to include formal integral values tardis-sn/tardis-refdata#41

Merged

empty commit to rerun tests

a2ba449

andrewfullard approved these changes Apr 14, 2021

View reviewed changes

andrewfullard marked this pull request as ready for review April 14, 2021 14:37

andrewfullard merged commit 6816e21 into tardis-sn:master Apr 14, 2021

atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021

Enable full tests for Formal Integral (tardis-sn#1499)

427a9c6

* remove pytest mark for skipping the tests * set overwrite option to True * empty commit to rerun tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Enable full tests for Formal Integral #1499

[WIP] Enable full tests for Formal Integral #1499

aribalam commented Mar 19, 2021 •

edited

Loading

andrewfullard commented Mar 19, 2021

aribalam commented Mar 20, 2021

andrewfullard commented Mar 22, 2021

aribalam commented Mar 22, 2021

epassaro commented Mar 22, 2021 •

edited

Loading

aribalam commented Mar 23, 2021

epassaro commented Mar 23, 2021

andrewfullard commented Mar 24, 2021

aribalam commented Mar 26, 2021 •

edited

Loading

andrewfullard commented Apr 9, 2021

codecov bot commented Apr 9, 2021

[WIP] Enable full tests for Formal Integral #1499

[WIP] Enable full tests for Formal Integral #1499

Conversation

aribalam commented Mar 19, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

andrewfullard commented Mar 19, 2021

aribalam commented Mar 20, 2021

andrewfullard commented Mar 22, 2021

aribalam commented Mar 22, 2021

epassaro commented Mar 22, 2021 • edited Loading

aribalam commented Mar 23, 2021

epassaro commented Mar 23, 2021

andrewfullard commented Mar 24, 2021

aribalam commented Mar 26, 2021 • edited Loading

andrewfullard commented Apr 9, 2021

codecov bot commented Apr 9, 2021

Codecov Report

aribalam commented Mar 19, 2021 •

edited

Loading

epassaro commented Mar 22, 2021 •

edited

Loading

aribalam commented Mar 26, 2021 •

edited

Loading