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

Added v_inner and v_outer, enable_nonhomologous_expansion for future use in nonhomologous expansion #2024

Merged
merged 16 commits into from
May 31, 2022

Conversation

sonachitchyan
Copy link
Member

@sonachitchyan sonachitchyan commented May 13, 2022

Description
This PR adds v_inner and v_outer to NumbaModel as arguments, taking them from MonteCarloRunner. This will be used in the future for the implementation of the nonhomologous expansion.

Motivation and context

How has this been tested?

  • Testing pipeline
  • Other

Examples

Type of change

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

Checklist

  • I have updated the documentation according to my changes
  • I have built the documentation by applying the build_docs label to this pull request (if you don't have enough privileges a reviewer will do it for you)
  • I have requested two reviewers for this pull request

@Rodot- Rodot- added the model label May 13, 2022
@Rodot-
Copy link
Contributor

Rodot- commented May 13, 2022

Please make sure your changes are PEP 8 compliant, you can just run black on the files you modified and it will do it for you

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #2024 (aa40494) into master (120b303) will decrease coverage by 0.13%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #2024      +/-   ##
==========================================
- Coverage   60.01%   59.87%   -0.14%     
==========================================
  Files          70       70              
  Lines        8115     8157      +42     
==========================================
+ Hits         4870     4884      +14     
- Misses       3245     3273      +28     
Impacted Files Coverage Δ
tardis/montecarlo/montecarlo_numba/base.py 29.75% <ø> (-2.07%) ⬇️
...dis/montecarlo/montecarlo_numba/formal_integral.py 53.87% <ø> (ø)
...dis/montecarlo/montecarlo_numba/numba_interface.py 34.90% <33.33%> (-1.32%) ⬇️
tardis/montecarlo/base.py 87.65% <100.00%> (+0.05%) ⬆️
tardis/io/atom_data/base.py 81.81% <0.00%> (-7.90%) ⬇️
tardis/io/atom_data/atom_web_download.py 40.00% <0.00%> (-2.11%) ⬇️
tardis/plasma/properties/continuum_processes.py 35.75% <0.00%> (ø)
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@sonachitchyan
Copy link
Member Author

@wkerzendorf when you have a chance, can you look through this? I think it's ready to be merged.

@Rodot- Rodot- requested a review from andrewfullard May 19, 2022 18:19
Copy link
Member

@wkerzendorf wkerzendorf left a comment

Choose a reason for hiding this comment

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

Please change as discussed

@@ -117,7 +117,7 @@ def formal_integral_model(request):
This gets the Numba model to be used in later tests
"""
r = request.param["r"]
model = NumbaModel(r[:-1], r[1:], 1 / c.c.cgs.value)
model = NumbaModel(r[:-1], r[1:], r[:-1], r[1:], 1 / c.c.cgs.value)
Copy link
Member

Choose a reason for hiding this comment

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

please change to velocities

@sonachitchyan sonachitchyan requested a review from wkerzendorf May 24, 2022 18:21
@Rodot- Rodot- self-requested a review May 26, 2022 02:12
@wkerzendorf wkerzendorf merged commit b237d51 into tardis-sn:master May 31, 2022
epassaro pushed a commit to epassaro/tardis that referenced this pull request Jun 2, 2022
…use in nonhomologous expansion (tardis-sn#2024)

* added v_outer and v_inner to NumbaModel

* defined default value for v_inner and v_outer in NumbaModel

* added enable_nonhomologous_expansion to io/schemas/montecarlo.yml

* added v_inner and v_outer to NumbaPlasma

* added v_inner and v_outer as arguments for NumbaModel in run() in montecarlo_numba

* added v_outer_cgs to MonteCarloRunner

* removed a comment

* ran black on the changed files

* added right arguments in test

* added v_inner, v_outer to tests

* added temporary fix to formal_integral.py

* ran black on modified files

* changed dummy values in the test

* Reverted content of file.

* made needed change in test_cuda_formal_integral.py

* made needed change in test_numba_formal_integral.py
epassaro added a commit to epassaro/tardis that referenced this pull request Jun 2, 2022
* Fix bot aliases in .mailmap (tardis-sn#2038)

* Added v_inner and v_outer, enable_nonhomologous_expansion for future use in nonhomologous expansion (tardis-sn#2024)

* added v_outer and v_inner to NumbaModel

* defined default value for v_inner and v_outer in NumbaModel

* added enable_nonhomologous_expansion to io/schemas/montecarlo.yml

* added v_inner and v_outer to NumbaPlasma

* added v_inner and v_outer as arguments for NumbaModel in run() in montecarlo_numba

* added v_outer_cgs to MonteCarloRunner

* removed a comment

* ran black on the changed files

* added right arguments in test

* added v_inner, v_outer to tests

* added temporary fix to formal_integral.py

* ran black on modified files

* changed dummy values in the test

* Reverted content of file.

* made needed change in test_cuda_formal_integral.py

* made needed change in test_numba_formal_integral.py

* Update README.rst in post-release (tardis-sn#2037)

* Fix docs badge

* Add license badge

* First attempt of creating credits.rst and README.rst via templates

* Working version

* Restore old README.rst

* Update post-release workflow

* Change .gitignore

* Various fixes

* Check PEP8 on CI (tardis-sn#2039)

* Add Flake8 job

* Change to file description

* Test payload

Co-authored-by: Sona Chitchyan <chitchyan.sona@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants