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

[MRG] Using Mamba in Pipelines #1210

Merged
merged 21 commits into from
Jul 19, 2020

Conversation

kaushik94
Copy link
Contributor

Do not merge this PR, this is a WIP. Checking if mamba installation is successful on the Azure pipelines.

@kaushik94
Copy link
Contributor Author

Looks like I have to add mamba to PATH.

@kaushik94 kaushik94 changed the title testing mamba installation. Simply install mamba from conda and try t… [WIP: DO NOT MERGE] Testing mamba installation Jun 28, 2020
@kaushik94 kaushik94 marked this pull request as draft June 28, 2020 02:09
@epassaro
Copy link
Member

Looks like I have to add mamba to PATH.

I think the problem is you're calling conda before installing Miniconda.

@epassaro
Copy link
Member

Remember the mamba binary lives in the Miniconda base environment.

@codecov
Copy link

codecov bot commented Jun 29, 2020

Codecov Report

Merging #1210 into master will increase coverage by 2.91%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1210      +/-   ##
==========================================
+ Coverage   77.71%   80.63%   +2.91%     
==========================================
  Files          91       41      -50     
  Lines        5727     3424    -2303     
==========================================
- Hits         4451     2761    -1690     
+ Misses       1276      663     -613     
Impacted Files Coverage Δ
tardis/scripts/cmfgen2tardis.py 0.00% <0.00%> (ø)
tardis/tests/integration_tests/runner.py
tardis/stats/base.py
tardis/gui/tests/test_gui.py
tardis/tests/test_util.py
tardis/io/tests/test_csvy_reader.py
tardis/model/tests/test_base.py
tardis/tests/integration_tests/conftest.py
tardis/montecarlo/tests/test_formal_integral.py
...s/plasma/tests/test_tardis_model_density_config.py
... and 47 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 f47075d...f649aa4. Read the comment docs.

@kaushik94 kaushik94 marked this pull request as ready for review July 1, 2020 15:46
@kaushik94 kaushik94 changed the title [WIP: DO NOT MERGE] Testing mamba installation [WIP] Testing mamba installation Jul 1, 2020
displayName: Add conda to PATH

- bash: |
sudo chown -R $USER $CONDA
# conda update -y conda
sudo chown -R $USER $CONDA
Copy link
Member

@epassaro epassaro Jul 3, 2020

Choose a reason for hiding this comment

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

Split this step in two steps. The first one does the chown thing and must run exclusive for macOS. The second one does the update, but comment it out because we don't use that unless is necessary.

displayName: "TARDIS test"

- script: |
Copy link
Member

Choose a reason for hiding this comment

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

Use bash instead of script.

displayName: "Install TARDIS env"

- bash: |
sh ci-helpers/fetch_reference_data.sh
# sh ci-helpers/fetch_reference_data.sh
Copy link
Member

Choose a reason for hiding this comment

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

Remove all the comments and the last cd line.

@kaushik94 kaushik94 changed the title [WIP] Testing mamba installation [MRG] Testing mamba installation Jul 3, 2020
@@ -65,6 +65,7 @@ jobs:
git lfs pull --include="atom_data/chianti_He.h5" origin
git lfs pull --include="plasma_reference/" origin
git lfs pull --include="unit_test_data.h5" origin
git lfs pull --include="packet_unittest.h5" origin
echo MD5 `md5sum unit_test_data.h5`
Copy link
Member

Choose a reason for hiding this comment

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

add a colon after MD5

@kaushik94 kaushik94 changed the title [MRG] Testing mamba installation [WIP: DO NOT MERGE] Testing mamba installation Jul 4, 2020
@kaushik94 kaushik94 changed the title [WIP: DO NOT MERGE] Testing mamba installation [MRG] Using Mamba in Pipelines Jul 6, 2020
@kaushik94 kaushik94 changed the title [MRG] Using Mamba in Pipelines [WIP: DO NOT MERGE] Using Mamba in Pipelines Jul 8, 2020
@kaushik94 kaushik94 changed the title [WIP: DO NOT MERGE] Using Mamba in Pipelines [REVIEW] Using Mamba in Pipelines Jul 8, 2020
@kaushik94 kaushik94 changed the title [REVIEW] Using Mamba in Pipelines [TESTING: DO NOT MERGE] Using Mamba in Pipelines Jul 8, 2020
Added a step to fail build on error as suggested by epassaro
improved documentation to steps
@kaushik94 kaushik94 changed the title [TESTING: DO NOT MERGE] Using Mamba in Pipelines [MRG] Using Mamba in Pipelines Jul 16, 2020
@kaushik94 kaushik94 requested a review from epassaro July 16, 2020 14:04
Copy link
Member

@epassaro epassaro left a comment

Choose a reason for hiding this comment

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

Just to clarify: the bandwith of GitHub LFS is limited, so we must save as much as we can. And yes, it's faster.

Also try to roll back the changes in both helpers scripts, since we are not using them anymore. Probably will be deleted in the future.

@kaushik94
Copy link
Contributor Author

Also try to roll back the changes in both helpers scripts, since we are not using them anymore. Probably will be deleted in the future.

@epassaro so I believe you already did that with #1242 ?

@epassaro
Copy link
Member

I made some changes to the fetch refdata bash script. Probably is the one we will keep, the other two files are not needed anymore.

Keep only the changes in the YAML file in this PR.

@wkerzendorf wkerzendorf self-requested a review July 19, 2020 14:29
@wkerzendorf wkerzendorf merged commit 74ae0fe into tardis-sn:master Jul 19, 2020
atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021
* testing mamba installation. Simply install mamba from conda and try to create env

* added mamba after conda installation

* check if mamba installation succesful

* installing mamba before activating

* fix syntax errors

* Pin mamba to 0.4.0 and make pipelines exit on error with any bash command

* move tardis_env.sh into pipelines file

* fetch reference data fetching from bash scripts to azure pipelines file

* fix syntax error

* Added packet data

* Adding variable shellopts to check

* added shellopts=errexit so that build fails on non-zero error code

* testing if pipeline fails

* reverting changes, testing succesful

* remove unused lines

* testing, this should fail the pipeline

* Made all single lines where necessary
Added a step to fail build on error as suggested by epassaro
improved documentation to steps

* added back the git lfs pull code

* removed git lfs codes

* added git lfs to pipelines to make it faster
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants