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

Add Workflow to check compatibility of TARDIS and Carsus #313

Merged
merged 59 commits into from
Feb 22, 2023

Conversation

atharva-2001
Copy link
Member

@atharva-2001 atharva-2001 commented Jul 26, 2022

📝 Description

Type: 🚀 feature 🚦 testing

This PR adds a workflow to check if the current Carsus atomic file makes any difference to the TARDIS spectrum and other data.

The workflow takes the following steps to do this:

  • Clone TARDIS, Carsus and TARDIS refdata repositories.
  • Generate the Carsus atomic data and save it as an artifact.
  • Download the artifact in another job.
  • Replace the atom data used for testing with this artifact.
  • Create a copy of the reference data generated from tests.
  • Use the artifact(atom data) to generate new reference data.
  • Compare this reference data with the saved one using this notebook.

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe)
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request
  • I updated the documentation according to my changes
  • I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

@atharva-2001 atharva-2001 force-pushed the tardis_carsus_bridge branch from 3c8afc3 to b808e38 Compare July 26, 2022 07:22
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #313 (3a8c50c) into master (275c383) will decrease coverage by 1.19%.
The diff coverage is 76.34%.

@@            Coverage Diff             @@
##           master     #313      +/-   ##
==========================================
- Coverage   74.44%   73.25%   -1.20%     
==========================================
  Files          23       25       +2     
  Lines        2614     3174     +560     
==========================================
+ Hits         1946     2325     +379     
- Misses        668      849     +181     
Impacted Files Coverage Δ
carsus/io/chianti_/chianti_.py 85.80% <0.00%> (+19.36%) ⬆️
carsus/io/output/base.py 9.02% <5.49%> (-1.82%) ⬇️
carsus/io/zeta.py 27.65% <16.66%> (-2.35%) ⬇️
carsus/io/cmfgen/base.py 87.14% <85.45%> (+3.14%) ⬆️
carsus/io/cmfgen/util.py 94.11% <94.11%> (ø)
carsus/io/kurucz/gfall.py 95.11% <100.00%> (-0.13%) ⬇️
carsus/io/nist/ionization.py 97.63% <100.00%> (+1.10%) ⬆️
carsus/io/nist/weightscomp.py 99.01% <100.00%> (+1.84%) ⬆️
carsus/io/util.py 100.00% <100.00%> (ø)
... and 4 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@atharva-2001 atharva-2001 force-pushed the tardis_carsus_bridge branch from fb6c3b1 to 890c619 Compare July 28, 2022 08:45
@atharva-2001 atharva-2001 requested review from chvogl and epassaro July 28, 2022 14:46
@atharva-2001 atharva-2001 force-pushed the tardis_carsus_bridge branch 5 times, most recently from 16a76c3 to 612586f Compare November 5, 2022 16:32
Copy link
Contributor

@chvogl chvogl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Copy link
Member

@sonachitchyan sonachitchyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@atharva-2001 atharva-2001 mentioned this pull request Jan 12, 2023
6 tasks
@atharva-2001 atharva-2001 merged commit 7114302 into tardis-sn:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants