Add tests to SDEC plotter module #1752

jaladh-singhal · 2021-07-23T16:07:02Z

Adds tests for SDEC plots

Motivation and context

Currently there are no tests for sdec_plot.py due to which we have to rerun notebook to test if plots look ok whenever a change affecting SDEC plots is made.

How has this been tested?

Testing pipeline.
Other.

Examples

Type of change

Bug fix.
New feature.
Breaking change.
None of the above.

Checklist

My change requires a change to the documentation.
- I have updated the documentation accordingly.
- (optional) I have built the documentation on my fork following the instructions.
I have assigned and requested two reviewers for this pull request.

codecov · 2021-07-23T16:18:41Z

Codecov Report

Merging #1752 (6336d67) into master (69fac16) will increase coverage by 4.46%.
The diff coverage is n/a.

❗ Current head 6336d67 differs from pull request most recent head eddc6b2. Consider uploading reports for the commit eddc6b2 to get more accurate results

@@            Coverage Diff             @@
##           master    #1752      +/-   ##
==========================================
+ Coverage   57.65%   62.12%   +4.46%     
==========================================
  Files          66       66              
  Lines        6809     6809              
==========================================
+ Hits         3926     4230     +304     
+ Misses       2883     2579     -304

Impacted Files	Coverage Δ
tardis/tardis/simulation/base.py	`83.79% <0.00%> (+0.46%)`	⬆️
tardis/tardis/visualization/plot_util.py	`100.00% <0.00%> (+6.25%)`	⬆️
tardis/tardis/montecarlo/montecarlo_numba/base.py	`32.14% <0.00%> (+7.14%)`	⬆️
tardis/tardis/base.py	`75.00% <0.00%> (+17.85%)`	⬆️
tardis/tardis/visualization/tools/sdec_plot.py	`85.97% <0.00%> (+76.45%)`	⬆️

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 69fac16...eddc6b2. Read the comment docs.

tardis/visualization/tools/tests/test_sdec_plot.py

tardis/visualization/tools/sdec_plot.py

jaladh-singhal

This is some really impressive work @atharva-2001 🎉

I guess only 2 things are left:

testing generate_plot_mpl
pushing up test data to tardis-refdata (I guess this is the reason test pipeline is failing?)

tardis/visualization/tools/tests/test_sdec_plot.py

tardis-bot · 2021-12-14T10:28:19Z

Before a pull request is accepted, it must meet the following criteria:

Is the necessary information provided?
Is this a duplicate PR?
- If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
- If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
Does it pass existing tests and are new tests provided if required?
- The test coverage should not decrease, and for new features should be close to 100%.
Is the code tidy?
- No unnecessary print lines or code comments.

jaladh-singhal

Excellent work @atharva-2001 - the comments are very helpful for readability and thanks for adding docstrings.

It's ready to be merged once test pipeline passes. We need to debug why ref-data can't be found in pipeline. Meanwhile can you post output of your local tests run?

tardis/visualization/tools/tests/test_sdec_plot.py

jaladh-singhal · 2021-12-15T16:26:45Z

Also the black pipeline fails - can you please fix that @atharva-2001?

atharva-2001 · 2021-12-15T17:03:41Z

/azp run update-refdata

jaladh-singhal

Thanks @atharva-2001 - pipelines pass now!

jaladh-singhal · 2022-01-11T17:41:41Z

tardis/montecarlo/montecarlo_numba/base.py

@@ -277,7 +279,7 @@ def montecarlo_main_loop(
                continue
            v_packets_energy_hist[idx] += vpackets_energy[j]

-    if montecarlo_configuration.VPACKET_LOGGING:
+    if virtual_packet_logging:
        for vpacket_collection in vpacket_collections:


I believe this requires rebasing?

and move functions

… names, skip tests when refdata is egenerated

jaladh-singhal · 2022-01-11T18:31:30Z

/azp run compare-refdata

azure-pipelines · 2022-01-11T18:31:41Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2022-01-11T18:43:15Z

Build succeeded 0a1922e

Click here to see results.

andrewfullard · 2022-01-11T19:50:00Z

tardis/visualization/tools/tests/test_sdec_plot.py

+                name=subgroup_name,
+            )
+
+            self.hdf_file.create_carray(


This looks like it could be a loop

yes, you're right

jaladh-singhal added priority - high 🔥 visualization labels Jul 23, 2021

jaladh-singhal assigned atharva-2001 Jul 23, 2021

atharva-2001 force-pushed the sdec-tests branch from 8383575 to 92c3513 Compare September 17, 2021 08:17

atharva-2001 force-pushed the sdec-tests branch from 92c3513 to f6be332 Compare November 6, 2021 15:32

atharva-2001 added the work in progress 🚧 label Nov 6, 2021

jaladh-singhal commented Nov 12, 2021

View reviewed changes

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

atharva-2001 force-pushed the sdec-tests branch from ec9b2f2 to 4064a23 Compare November 22, 2021 13:44

atharva-2001 reviewed Nov 23, 2021

View reviewed changes

tardis/visualization/tools/sdec_plot.py Outdated Show resolved Hide resolved

atharva-2001 mentioned this pull request Nov 25, 2021

Name plotly traces in SDEC plot #1830

Merged

10 tasks

jaladh-singhal commented Nov 26, 2021

View reviewed changes

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

atharva-2001 force-pushed the sdec-tests branch 2 times, most recently from 4064a23 to 1ee1d42 Compare December 1, 2021 17:40

atharva-2001 force-pushed the sdec-tests branch 3 times, most recently from 211ada4 to d2c92be Compare December 11, 2021 09:52

atharva-2001 marked this pull request as ready for review December 11, 2021 12:51

atharva-2001 force-pushed the sdec-tests branch 4 times, most recently from 9208be0 to 4bad25a Compare December 14, 2021 08:08

atharva-2001 mentioned this pull request Dec 14, 2021

Add SDEC reference data tardis-sn/tardis-refdata#48

Merged

10 tasks

atharva-2001 requested review from jamesgillanders, MarkMageeAstro and marxwillia December 14, 2021 10:27

jaladh-singhal commented Dec 15, 2021

View reviewed changes

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

tardis/visualization/tools/tests/test_sdec_plot.py Outdated Show resolved Hide resolved

jaladh-singhal commented Jan 11, 2022

View reviewed changes

jaladh-singhal and others added 19 commits January 11, 2022 23:51

Created tests dir

2fbdec6

Added a fixture to instantiate SDECPlotter

21bf2a3

Added comments, todos & reduced spectrum's num of bins

40a5793

Check plotly traces and add fixture returning config_verysimple

726e00d

Add tests for _calculate_plotting_data function

f80019e

Save plotter data in hdf file

3878ae7

Retrieve test data

828e378

and move functions

Format using black and remove unused imports

33c3165

Test species list

bc3a190

Test x and y values of plotly traces

6a01837

Add basic matplotlib tests and format using black

6148414

Add tests for observed spectrum

e222036

Fix reference data path

37f7f85

Convert lum_to_flux to cgs value if needed before saving

d5a3c6d

Add docstrings

b41a558

Add species_list and nelements to generate plot tests

5ee9602

Use PyTables instead of h5py

321b360

Set number of virtual packets

3d81353

Sanitize test id to make valid group names, test stackgroup and trace…

eddc6b2

… names, skip tests when refdata is egenerated

jaladh-singhal force-pushed the sdec-tests branch from 5f82d0a to eddc6b2 Compare January 11, 2022 18:22

atharva-2001 approved these changes Jan 11, 2022

View reviewed changes

andrewfullard reviewed Jan 11, 2022

View reviewed changes

andrewfullard approved these changes Jan 11, 2022

View reviewed changes

andrewfullard merged commit 8c21bc7 into tardis-sn:master Jan 11, 2022

atharva-2001 removed the work in progress 🚧 label Jan 23, 2022

atharva-2001 mentioned this pull request Jan 24, 2022

debugging #1752 #1845

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests to SDEC plotter module #1752

Add tests to SDEC plotter module #1752

jaladh-singhal commented Jul 23, 2021 •

edited

Loading

codecov bot commented Jul 23, 2021 •

edited

Loading

jaladh-singhal left a comment •

edited

Loading

tardis-bot commented Dec 14, 2021

jaladh-singhal left a comment

jaladh-singhal commented Dec 15, 2021

atharva-2001 commented Dec 15, 2021

jaladh-singhal left a comment

jaladh-singhal Jan 11, 2022

jaladh-singhal commented Jan 11, 2022

azure-pipelines bot commented Jan 11, 2022

azure-pipelines bot commented Jan 11, 2022

andrewfullard Jan 11, 2022

atharva-2001 Jan 11, 2022 •

edited

Loading

Add tests to SDEC plotter module #1752

Add tests to SDEC plotter module #1752

Conversation

jaladh-singhal commented Jul 23, 2021 • edited Loading

codecov bot commented Jul 23, 2021 • edited Loading

Codecov Report

jaladh-singhal left a comment • edited Loading

Choose a reason for hiding this comment

tardis-bot commented Dec 14, 2021

jaladh-singhal left a comment

Choose a reason for hiding this comment

jaladh-singhal commented Dec 15, 2021

atharva-2001 commented Dec 15, 2021

jaladh-singhal left a comment

Choose a reason for hiding this comment

jaladh-singhal Jan 11, 2022

Choose a reason for hiding this comment

jaladh-singhal commented Jan 11, 2022

azure-pipelines bot commented Jan 11, 2022

azure-pipelines bot commented Jan 11, 2022

andrewfullard Jan 11, 2022

Choose a reason for hiding this comment

atharva-2001 Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

jaladh-singhal commented Jul 23, 2021 •

edited

Loading

codecov bot commented Jul 23, 2021 •

edited

Loading

jaladh-singhal left a comment •

edited

Loading

atharva-2001 Jan 11, 2022 •

edited

Loading