Opacity state slice #2758

Rodot- · 2024-07-29T17:59:29Z

📝 Adds a geitem operation to the opacity state class which allows slicing and indexing by shell

Type: 🚀 feature
The opacity state should be able to be sliced in order to run the Montecarlo on sub-slices of the geometry. Let me know if there are other features or considerations to think about.

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

…lver to use the bounds of the gemoetry indices

tardis-bot · 2024-07-29T17:59:53Z

*beep* *bop*
Hi human,
I ran ruff on the latest commit (df19bae).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Complete output(might be large):

codecov · 2024-07-29T18:22:24Z

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 69.35%. Comparing base (5e7a23b) to head (df19bae).
Report is 5 commits behind head on master.

Files	Patch %	Lines
tardis/opacities/opacity_state.py	50.00%	1 Missing ⚠️
tardis/transport/montecarlo/numba_interface.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2758      +/-   ##
==========================================
- Coverage   69.83%   69.35%   -0.48%     
==========================================
  Files         191      196       +5     
  Lines       15022    15011      -11     
==========================================
- Hits        10490    10411      -79     
- Misses       4532     4600      +68

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ate object defined in the opacities module

andrewfullard

I hate that this is in 2 places but oh well. Please remove the print statements from the test.

wkerzendorf · 2024-07-29T18:01:22Z

tardis/transport/montecarlo/numba_interface.py

@@ -112,6 +112,56 @@ def __init__(
        self.photo_ion_activation_idx = photo_ion_activation_idx
        self.k_packet_idx = k_packet_idx

+    def __getitem__(self, i):


I thought we can't make new classes in a jitclass?

I was mistaken

wkerzendorf · 2024-07-30T19:34:48Z

tardis/opacities/opacity_state.py

+        """
+        # NOTE: This currently will not work with continuum processes since it does not slice those arrays
+        return OpacityState(
+            self.electron_density[i],


check if i is an int and then raise an error

This wouldn't allow one to slice the opacity state then

To be clear, Wolfgang is concerned about what happens if somebody requests OpacityState[5] and passes that to something else.

yep. OpacityState[5] should return either a length 1 OPacity state or fail

Which do you prefer?

andrewfullard

I'm happy with this.

* added setup function Signed-off-by: Asish Kumar <[email protected]> * refactor geometry calculate distances Signed-off-by: Asish Kumar <[email protected]> * Refactor montecarlo estimators in benchmark code Signed-off-by: Asish Kumar <[email protected]> * add cache and setup functions Signed-off-by: Asish Kumar <[email protected]> * add cache and setup functions Signed-off-by: Asish Kumar <[email protected]> * ran ruff linter Signed-off-by: Asish Kumar <[email protected]> * Thou hath vanquished the redundant variables and functions Signed-off-by: Asish Kumar <[email protected]> * fix bugs Signed-off-by: Asish Kumar <[email protected]> * change benchmark yml Signed-off-by: Asish Kumar <[email protected]> * repeat in class Signed-off-by: Asish Kumar <[email protected]> * Benchmark run_tardis with track_rpacket enabled (tardis-sn#2720) * Benchmark run_tardis with track_rpacket enabled * Update config to resemble time_run_tardis config * Fix Benchmark error * Cache Benchmark Property * Remove Cached Property decorator * rounds in benchmarks Signed-off-by: Asish Kumar <[email protected]> * fix Signed-off-by: Asish Kumar <[email protected]> * fix error in formal integral Signed-off-by: Asish Kumar <[email protected]> * more cache Signed-off-by: Asish Kumar <[email protected]> * sort by ratio Signed-off-by: Asish Kumar <[email protected]> * Donot run docs, benchmarks and stardis tests on draft PRs. (tardis-sn#2761) * Add ready to review to run tests * Add if condition to check PR status * Running stardis tests only on push to master * Removing osx-64 from workflows * Add Extend Array Function (tardis-sn#2771) * Add Extend Array Function * Fix Typo and restructure * Restructure * Doc string update * Was expecting numpy arrays to be passed by reference but it is not so updating it * Add test for the extend array function * Opacity state slice (tardis-sn#2758) * Added basic sliing operation to opacitystate and set the transport solver to use the bounds of the gemoetry indices * Reformatted the slicing index * Added slicing operation with __getitem__ * Added docstring * removed old slice method * Ran black * Added back the 'slice' method because __getitem__ is wacky * added missing argument * fixed missing argument * ran black * fixed the tests * ran black * Updated the slice to use __getitem__ again and modified the opacitystate object defined in the opacities module * Removed old slice method and print statements * removed print statement from the test * added type annotation to OpacityState getitem * updated numba interface * Fixes black formatting * Release Pipeline Fixes (tardis-sn#2767) * Fix zenodo badge * Pick from the website * update badge in credits.rst * Use the all versions DOI * Inherit secrets * Change triggers for testing * Reset Triggers * Fix ruff comments issue (tardis-sn#2762) * Fix ruff comments issue * Only check files changed * Testing triggers * Fix bash commands * No pager * fetch depth * Reset all changes * cache in setup function Signed-off-by: Asish Kumar <[email protected]> * change benchmark yml Signed-off-by: Asish Kumar <[email protected]> --------- Signed-off-by: Asish Kumar <[email protected]> Co-authored-by: Sumit Gupta <[email protected]> Co-authored-by: KasukabeDefenceForce <[email protected]> Co-authored-by: Jack O'Brien <[email protected]> Co-authored-by: Andrew Fullard <[email protected]> Co-authored-by: Atharva Arya <[email protected]>

Rodot- added 3 commits July 29, 2024 13:42

Added basic sliing operation to opacitystate and set the transport so…

7773ff4

…lver to use the bounds of the gemoetry indices

Reformatted the slicing index

322389d

Added slicing operation with __getitem__

76a2602

Rodot- marked this pull request as draft July 29, 2024 17:59

Rodot- requested review from wkerzendorf and Knights-Templars July 29, 2024 18:00

Rodot- added 3 commits July 29, 2024 14:07

Added docstring

54cf624

removed old slice method

4a82dca

Ran black

c441e43

Rodot- added 8 commits July 29, 2024 16:00

Added back the 'slice' method because __getitem__ is wacky

04691d9

added missing argument

834f626

fixed missing argument

12eb265

ran black

d8ccbac

fixed the tests

54ab779

ran black

dd3c274

Updated the slice to use __getitem__ again and modified the opacityst…

213eeb4

…ate object defined in the opacities module

Removed old slice method and print statements

9535989

Rodot- marked this pull request as ready for review July 30, 2024 14:21

andrewfullard requested changes Jul 30, 2024

View reviewed changes

removed print statement from the test

43a9762

Rodot- requested a review from andrewfullard July 30, 2024 17:20

andrewfullard previously approved these changes Jul 30, 2024

View reviewed changes

wkerzendorf reviewed Jul 30, 2024

View reviewed changes

added type annotation to OpacityState getitem

401c6c5

Rodot- dismissed andrewfullard’s stale review via 401c6c5 July 31, 2024 14:37

updated numba interface

df19bae

Rodot- requested review from wkerzendorf and andrewfullard July 31, 2024 14:57

andrewfullard approved these changes Aug 1, 2024

View reviewed changes

wkerzendorf merged commit c82dde5 into tardis-sn:master Aug 1, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opacity state slice #2758

Opacity state slice #2758

Rodot- commented Jul 29, 2024 •

edited

Loading

tardis-bot commented Jul 29, 2024 •

edited

Loading

codecov bot commented Jul 29, 2024 •

edited

Loading

andrewfullard left a comment

wkerzendorf Jul 29, 2024

Rodot- Jul 30, 2024

wkerzendorf Jul 30, 2024

Rodot- Jul 30, 2024

andrewfullard Jul 30, 2024

wkerzendorf Jul 31, 2024

Rodot- Jul 31, 2024

andrewfullard left a comment

Opacity state slice #2758

Opacity state slice #2758

Conversation

Rodot- commented Jul 29, 2024 • edited Loading

📝 Adds a geitem operation to the opacity state class which allows slicing and indexing by shell

🚦 Testing

☑️ Checklist

tardis-bot commented Jul 29, 2024 • edited Loading

codecov bot commented Jul 29, 2024 • edited Loading

Codecov Report

andrewfullard left a comment

Choose a reason for hiding this comment

wkerzendorf Jul 29, 2024

Choose a reason for hiding this comment

Rodot- Jul 30, 2024

Choose a reason for hiding this comment

wkerzendorf Jul 30, 2024

Choose a reason for hiding this comment

Rodot- Jul 30, 2024

Choose a reason for hiding this comment

andrewfullard Jul 30, 2024

Choose a reason for hiding this comment

wkerzendorf Jul 31, 2024

Choose a reason for hiding this comment

Rodot- Jul 31, 2024

Choose a reason for hiding this comment

andrewfullard left a comment

Choose a reason for hiding this comment

Rodot- commented Jul 29, 2024 •

edited

Loading

tardis-bot commented Jul 29, 2024 •

edited

Loading

codecov bot commented Jul 29, 2024 •

edited

Loading