Cache LFS Objects in the TARDIS-Carsus Compatibility Check Workflow #327

atharva-2001 · 2023-01-27T10:35:39Z

📝 Description

📌 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.

change env file path change env file path change path again change path again skip first step path again Package path

Fix atom data path

repo path test

Fix package path

Fix notebook path Fix notebook path Require carsus build to run again before running tardis build

Fix output format of atom data notebook Fix atom data path again Print atom data path Fix atom data path

…the new atomic data file

Fix atomic data path

review-notebook-app · 2023-01-27T10:35:43Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2023-01-27T10:53:29Z

Codecov Report

Merging #327 (01d61af) into master (275c383) will decrease coverage by 1.09%.
The diff coverage is 62.64%.

❗ Current head 01d61af differs from pull request most recent head c22ea9f. Consider uploading reports for the commit c22ea9f to get more accurate results

@@            Coverage Diff             @@
##           master     #327      +/-   ##
==========================================
- Coverage   74.44%   73.36%   -1.09%     
==========================================
  Files          23       25       +2     
  Lines        2614     3187     +573     
==========================================
+ Hits         1946     2338     +392     
- 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% <7.61%> (-1.82%)`	⬇️
carsus/io/atom_data_compare.py	`12.97% <12.97%> (ø)`
carsus/io/zeta.py	`27.65% <16.66%> (-2.35%)`	⬇️
carsus/io/cmfgen/base.py	`87.14% <85.49%> (+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.68% <100.00%> (+1.15%)`	⬆️
carsus/io/nist/weightscomp.py	`99.05% <100.00%> (+1.88%)`	⬆️
carsus/io/util.py	`100.00% <100.00%> (ø)`

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

epassaro · 2023-03-15T15:35:45Z

.github/workflows/bridge.yml

+        id: lfs-cache
+        with:
+          path: tardis-refdata/.git/lfs
+          key: ${{ runner.os }}-lfs-${{ hashFiles('tardis-refdata/.lfs-assets-id') }}-v1


why v1 is hardcoded?

In case we want to invalidate the cache manually- actions/checkout#165

we use a CACHE_NUMBER: 0 variable on top of the file for that (I think setting to 1 is better though) and then replace v1 with v${{ env.CACHE_NUMBER }}

I agree with @epassaro.

epassaro · 2023-03-15T15:37:41Z

Could you rebase this PR to master?

chvogl

Looks good to me.

atharva-2001 and others added 30 commits June 28, 2022 18:48

Initial commit

71f9239

change env file path change env file path change path again change path again skip first step path again Package path

Add step to download the CMFGEN database

46591b5

Add URLs to the CMFGEN database

f91b7fb

Fix atom data path

Add tardis build job

d8232b0

repo path test

Add strategy for TARDIS build job

e9487e4

Fix lock file path

3fd40c0

Fix package path

Add step to run notebook

0983b61

Upload spectrum notebook

0e69c0f

Fix notebook path Fix notebook path Require carsus build to run again before running tardis build

Fix atom data path

b211146

Fix output format of atom data notebook Fix atom data path again Print atom data path Fix atom data path

Change URL used to get the reference data file

3e1d648

Download the file from the workflow

db03395

Upload reference data as artifact

3715561

Try downloading all reference data

38a8ce3

Clone my fork of tardis refdata

6a419d7

Use lfs when cloning

85208f1

Separate commands

7ed5bca

Add new env variables, generate reference data for comparision using …

6a14e28

…the new atomic data file

Clone my fork with arraydiff unpinned

f0552c6

Fix atomic data path

Fix notebook path when creating artifact

cda1c5e

Fix paths in environment variables

c100e69

Install bokeh

b808e38

Fix paths after moving branch

0eed8c0

Rename notebook

79a26cf

Remove variables related to matrix strategy

890c619

Remove step to download reference data

40b8496

Create copy of the quickstart notebook

fbf5c56

Delete markdown

b8129a3

Add refdata_gen notebook instead of quickstart

b820d3f

Change branch when cloning carsus in the workflow

f0abdf9

Try removing chianti reader

6fb9add

atharva-2001 added 13 commits October 20, 2022 19:55

Change file name in notebook

8f920ad

Delete lines commented out which are not necessary

f9b7183

Do not clone my fork of the carsus repository

0da5cbb

Change title of the notebook and rename it

57be609

Change file and step names

9eb8aa8

No need for inputs

db6cfd2

Check file contents according to changes in tardis-sn/tardis-refdata#58

80d1417

Change path of refdata_compare_result file

dafafd3

Change trigger back to workflow_dispatch

6fa893a

Update URL to use TARDIS fork

c7aaaeb

Do not pin bokeh

a5cd46a

Add name to comparison notebook artifact

3a8c50c

Cache LFS objects

90e8ac7

atharva-2001 added 2 commits February 23, 2023 18:58

Do git lfs checkout if the cache key is found

5cb8396

Merge branch 'master' into cache_LFS_bridge

07f4570

jamesgillanders self-requested a review March 9, 2023 17:20

atharva-2001 force-pushed the cache_LFS_bridge branch from 76bc01b to 07f4570 Compare March 12, 2023 09:52

epassaro reviewed Mar 15, 2023

View reviewed changes

atharva-2001 force-pushed the cache_LFS_bridge branch from 01d61af to 07f4570 Compare March 16, 2023 14:45

atharva-2001 marked this pull request as ready for review March 16, 2023 14:50

atharva-2001 requested review from epassaro, chvogl and sonachitchyan March 23, 2023 11:15

chvogl approved these changes Mar 23, 2023

View reviewed changes

Use cache no instead of v1 hardcoding

c22ea9f

andrewfullard approved these changes Mar 30, 2023

View reviewed changes

andrewfullard merged commit 59a76e4 into tardis-sn:master Mar 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache LFS Objects in the TARDIS-Carsus Compatibility Check Workflow #327

Cache LFS Objects in the TARDIS-Carsus Compatibility Check Workflow #327

atharva-2001 commented Jan 27, 2023

review-notebook-app bot commented Jan 27, 2023

codecov bot commented Jan 27, 2023 •

edited

Loading

epassaro Mar 15, 2023

atharva-2001 Mar 23, 2023

epassaro Mar 23, 2023

chvogl Mar 23, 2023

epassaro commented Mar 15, 2023

chvogl left a comment

Cache LFS Objects in the TARDIS-Carsus Compatibility Check Workflow #327

Cache LFS Objects in the TARDIS-Carsus Compatibility Check Workflow #327

Conversation

atharva-2001 commented Jan 27, 2023

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

review-notebook-app bot commented Jan 27, 2023

codecov bot commented Jan 27, 2023 • edited Loading

Codecov Report

epassaro Mar 15, 2023

Choose a reason for hiding this comment

atharva-2001 Mar 23, 2023

Choose a reason for hiding this comment

epassaro Mar 23, 2023

Choose a reason for hiding this comment

chvogl Mar 23, 2023

Choose a reason for hiding this comment

epassaro commented Mar 15, 2023

chvogl left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 27, 2023 •

edited

Loading