Skip to content

Conversation

@wkerzendorf
Copy link
Member

Refactor the opacity state initialization to utilize a numba implementation for improved performance and update related tests. Additionally, streamline imports and introduce a new LineInteractionType class for better organization.

Copilot AI review requested due to automatic review settings August 26, 2025 21:08
@tardis-bot
Copy link
Contributor

tardis-bot commented Aug 26, 2025

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

Details
2	I001  	[*] unsorted-imports
1	F401  	[*] unused-import
1	RET506	[*] superfluous-else-raise
Found 4 errors.
[*] 4 fixable with the `--fix` option.

Complete output(might be large):

Details
benchmarks/benchmark_base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
benchmarks/benchmark_base.py:74:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/transport/montecarlo/tests/conftest.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/tests/conftest.py:7:44: F401 [*] `tardis.opacities.opacity_state.opacity_state_initialize` imported but unused
Found 4 errors.
[*] 4 fixable with the `--fix` option.

This comment was marked as outdated.

@wkerzendorf wkerzendorf requested a review from Copilot August 26, 2025 21:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors opacity state handling by migrating the OpacityState class to a new numba-based implementation and reorganizes imports for better code organization. The changes improve performance through numba optimization while maintaining the same functionality.

  • Migrates opacity state initialization from numba_interface to a dedicated opacity_state_numba module
  • Moves LineInteractionType enum from numba_interface to the interaction module
  • Updates all import statements and function calls to use the new module locations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tardis/opacities/opacity_state_numba.py Replaces old spec-based jitclass with modern type annotations and improves documentation
tardis/transport/montecarlo/interaction.py Adds LineInteractionType enum definition and removes old import
tardis/transport/montecarlo/tests/test_interaction.py Updates import to use LineInteractionType from new location
tardis/transport/montecarlo/tests/conftest.py Updates import and function call to use new opacity state initialization
tardis/transport/montecarlo/configuration/base.py Updates import to use LineInteractionType from new location
benchmarks/benchmark_base.py Updates import and function call to use new opacity state initialization

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

]
@jitclass
class OpacityStateNumba:
electron_density: nb.float64[:] # type: ignore[misc]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what those comments are for

Copy link
Member Author

Choose a reason for hiding this comment

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

to make sure the lint4er doesn't go nuts

@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.92%. Comparing base (f70fee8) to head (f9a12fa).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
tardis/opacities/opacity_state_numba.py 93.54% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3264      +/-   ##
==========================================
- Coverage   74.11%   73.92%   -0.19%     
==========================================
  Files         259      259              
  Lines       17866    17886      +20     
==========================================
- Hits        13241    13223      -18     
- Misses       4625     4663      +38     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wkerzendorf wkerzendorf merged commit 995879b into tardis-sn:master Aug 27, 2025
22 of 24 checks passed
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.

3 participants