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

Change virtual spectrum spawn range start value to 1 #1794

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

atharva-2001
Copy link
Member

@atharva-2001 atharva-2001 commented Sep 10, 2021

This PR fixes the following warning:

[py.warnings][WARNING]  /home/atharva/miniconda3/envs/tardis/lib/python3.7/site-packages/astropy/units/equivalencies.py:124: RuntimeWarning: divide by zero encountered in double_scalars
  (si.m, si.Hz, lambda x: _si.c.value / x),
 (warnings.py:110)

by setting the virtual_spectrum_spawn_range in the montecarlo.yml file from 0 to 1.

Description

The error is caused due to these lines as mentioned here

    montecarlo_configuration.v_packet_spawn_end_frequency = (
        runner.virtual_spectrum_spawn_range.start.to(
            u.Hz, equivalencies=u.spectral()
        ).value
    )

because we are converting wavelength to frequency and the wavelength is zero. Any other number apart from 0 here should fix the warning.
The same warning can be replicated using this code:

import astropy.units as u
a = 0 * u.AA
a.to(u.Hz, equivalencies=u.spectral()).value

Motivation and context

Resolves #1724

How has this been tested?

  • Testing pipeline.
  • Other.

Examples

Removing this warning also allows progress bars to update seamlessly in the console, without having to disable the warnings. Earlier the console output was something like this:
image

Now the progress bars update seamlessly because the warning is not displayed with each iteration.

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.

@tardis-bot
Copy link
Contributor

tardis-bot commented Sep 10, 2021

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.

@codecov
Copy link

codecov bot commented Sep 10, 2021

Codecov Report

Merging #1794 (0108fec) into master (6a0b8db) will not change coverage.
The diff coverage is n/a.

❗ Current head 0108fec differs from pull request most recent head 0e0d0ad. Consider uploading reports for the commit 0e0d0ad to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1794   +/-   ##
=======================================
  Coverage   58.45%   58.45%           
=======================================
  Files          66       66           
  Lines        6687     6687           
=======================================
  Hits         3909     3909           
  Misses       2778     2778           

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 6a0b8db...0e0d0ad. Read the comment docs.

@jaladh-singhal jaladh-singhal merged commit e6a8af2 into tardis-sn:master Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants