Skip to content

[GSOC]-Added S92Reader for Shull & Van Steenberg photoionization data #443

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sundanc
Copy link

@sundanc sundanc commented Mar 31, 2025

Added Support for Shull & Van Steenberg 1992 Photoionization Data

📝 Description

Type: 🚀 feature

This PR adds functionality to read, process, and incorporate photoionization cross-section data into the TARDIS atomic dataset. It includes a new S92Reader class that can parse the s92.201 data file, calculate cross-sections using the Verner & Yakovlev fitting formula, and integrate with the existing TARDISAtomData class.

I'm implementing this feature as part of my GSoC 2025 project "Continuum opacity source reader." aka my StarDust Alchemist. This is the first step in adding support for additional continuum opacity sources to TARDIS.

Key features:

  • New module carsus.io.s92 with S92Reader class for parsing S92 data files
  • Integration with TARDISAtomData via the _create_photoionization_data method
  • Automatic downloading of the S92 data file from Harvard FTP when needed
  • Comprehensive test suite for the new functionality
  • Documentation with usage examples

I look forward to improving this implementation with:

  • More robust file format handling
  • Support for additional photoionization data sources
  • Memory optimization for large datasets
  • Enhanced error handling and validation

📌 Resources

🚦 Testing

  • Testing pipeline
    • Created comprehensive test suite for the S92Reader functionality
    • Tests verify file parsing, cross-section calculation, and integration with TARDISAtomData
  • Other method
    • Manually tested with downloaded s92.201 data
    • Verified cross-sections match expected values from the literature

☑️ 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

Copy link
Contributor

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

150	    	[ ] syntax-error
1	F401	[*] unused-import

Complete output(might be large):

carsus/io/s92/base.py:10:28: F401 [*] `carsus.io.base.BaseParser` imported but unused
docs/io/io.rst:1:1: SyntaxError: Expected a statement
docs/io/io.rst:1:2: SyntaxError: Expected a statement
docs/io/io.rst:1:8: SyntaxError: Expected an expression
docs/io/io.rst:3:1: SyntaxError: Expected an expression
docs/io/io.rst:3:3: SyntaxError: Expected an expression
docs/io/io.rst:3:5: SyntaxError: Expected an expression
docs/io/io.rst:3:7: SyntaxError: Expected an expression
docs/io/io.rst:3:9: SyntaxError: Expected an expression
docs/io/io.rst:3:11: SyntaxError: Expected an expression
docs/io/io.rst:3:13: SyntaxError: Expected an expression
docs/io/io.rst:3:15: SyntaxError: Expected an expression
docs/io/io.rst:3:17: SyntaxError: Expected an expression
docs/io/io.rst:3:19: SyntaxError: Starred expression cannot be used here
docs/io/io.rst:3:20: SyntaxError: Expected an expression
docs/io/io.rst:5:1: SyntaxError: Expected an expression
docs/io/io.rst:5:3: SyntaxError: Expected an expression
docs/io/io.rst:5:5: SyntaxError: Expected an expression
docs/io/io.rst:5:7: SyntaxError: Expected an expression
docs/io/io.rst:5:9: SyntaxError: Expected an expression
docs/io/io.rst:5:11: SyntaxError: Expected an expression
docs/io/io.rst:5:13: SyntaxError: Expected an expression
docs/io/io.rst:5:15: SyntaxError: Expected an expression
docs/io/io.rst:5:17: SyntaxError: Expected an expression
docs/io/io.rst:5:19: SyntaxError: Starred expression cannot be used here
docs/io/io.rst:5:20: SyntaxError: Expected an expression
docs/io/io.rst:7:1: SyntaxError: Expected a statement
docs/io/io.rst:7:2: SyntaxError: Expected a statement
docs/io/io.rst:7:19: SyntaxError: Expected an expression
docs/io/io.rst:9:8: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:10:1: SyntaxError: Expected a statement
docs/io/io.rst:10:3: SyntaxError: Expected a statement
docs/io/io.rst:10:5: SyntaxError: Expected a statement
docs/io/io.rst:10:7: SyntaxError: Expected a statement
docs/io/io.rst:10:9: SyntaxError: Expected a statement
docs/io/io.rst:10:11: SyntaxError: Expected a statement
docs/io/io.rst:10:13: SyntaxError: Expected a statement
docs/io/io.rst:12:1: SyntaxError: Expected a statement
docs/io/io.rst:12:2: SyntaxError: Expected a statement
docs/io/io.rst:12:15: SyntaxError: Expected an expression
docs/io/io.rst:15:1: SyntaxError: Expected a statement
docs/io/io.rst:15:3: SyntaxError: Expected a statement
docs/io/io.rst:15:5: SyntaxError: Expected a statement
docs/io/io.rst:15:7: SyntaxError: Expected a statement
docs/io/io.rst:15:9: SyntaxError: Expected a statement
docs/io/io.rst:15:10: SyntaxError: Expected a statement
docs/io/io.rst:17:1: SyntaxError: Expected a statement
docs/io/io.rst:17:2: SyntaxError: Expected a statement
docs/io/io.rst:17:10: SyntaxError: Expected an expression
docs/io/io.rst:20:5: SyntaxError: Expected an expression
docs/io/io.rst:22:1: SyntaxError: Expected a statement
docs/io/io.rst:22:2: SyntaxError: Expected a statement
docs/io/io.rst:22:12: SyntaxError: Expected an expression
docs/io/io.rst:25:7: SyntaxError: Expected an expression
docs/io/io.rst:27:1: SyntaxError: Expected a statement
docs/io/io.rst:27:2: SyntaxError: Expected a statement
docs/io/io.rst:27:13: SyntaxError: Expected an expression
docs/io/io.rst:30:8: SyntaxError: Expected an expression
docs/io/io.rst:32:1: SyntaxError: Expected a statement
docs/io/io.rst:32:2: SyntaxError: Expected a statement
docs/io/io.rst:32:10: SyntaxError: Expected an expression
docs/io/io.rst:35:5: SyntaxError: Expected an expression
docs/io/io.rst:37:1: SyntaxError: Expected a statement
docs/io/io.rst:37:2: SyntaxError: Expected a statement
docs/io/io.rst:37:9: SyntaxError: Expected an expression
docs/io/io.rst:39:13: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:40:29: SyntaxError: Expected an expression
docs/io/io.rst:42:5: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:42:15: SyntaxError: Compound statements are not allowed on the same line as simple statements
docs/io/io.rst:42:30: SyntaxError: Expected ':', found name
docs/io/io.rst:42:44: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:42:47: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:42:64: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:42:80: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:43:12: SyntaxError: Expected 'import', found '&'
docs/io/io.rst:43:18: SyntaxError: Expected ',', found name
docs/io/io.rst:43:28: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:43:41: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:43:46: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:43:55: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:43:71: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:44:5: SyntaxError: Invalid assignment target
docs/io/io.rst:44:20: SyntaxError: Expected an identifier
docs/io/io.rst:44:20: SyntaxError: Expected 'in', found newline
docs/io/io.rst:46:1: SyntaxError: Expected an indented block after `for` statement
docs/io/io.rst:46:7: SyntaxError: Simple statements must be separated by newlines or semicolons
docs/io/io.rst:47:1: SyntaxError: Expected a statement
docs/io/io.rst:47:2: SyntaxError: Expected a statement
docs/io/io.rst:47:3: SyntaxError: Expected a statement
docs/io/io.rst:47:4: SyntaxError: Expected a statement
docs/io/io.rst:47:5: SyntaxError: Expected a statement
docs/io/io.rst:47:6: SyntaxError: Expected a statement
docs/io/io.rst:47:7: SyntaxError: Expected a statement
docs/io/io.rst:47:8: SyntaxError: Expected a statement
docs/io/io.rst:47:9: SyntaxError: Expected a statement
docs/io/io.rst:47:10: SyntaxError: Expected a statement
docs/io/io.rst:47:11: SyntaxError: Expected a statement
docs/io/io.rst:49:1: SyntaxError: Expected a statement
docs/io/io.rst:49:2: SyntaxError: Expected a statement
docs/io/io.rst:49:4: SyntaxError: Invalid annotated assignment target
docs/io/io.rst:49:15: SyntaxError: Expected an expression
docs/io/io.rst:51:1: SyntaxError: Unexpected indentation
docs/io/io.rst:69:1: SyntaxError: Expected a statement
docs/io/io.rst:69:13: SyntaxError: Compound statements are not allowed on the same line as simple statements
docs/io/io.rst:69:32: SyntaxError: Expected ',', found newline
docs/io/io.rst:70:1: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:2: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:3: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:4: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:5: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:6: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:7: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:8: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:9: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:10: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:11: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:12: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:13: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:14: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:15: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:16: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:17: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:18: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:19: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:20: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:21: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:22: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:23: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:24: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:25: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:26: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:27: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:28: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:29: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:30: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:31: SyntaxError: Expected ',', found '^'
docs/io/io.rst:70:32: SyntaxError: Expected ',', found newline
docs/io/io.rst:72:5: SyntaxError: Expected ',', found name
docs/io/io.rst:72:9: SyntaxError: Expected ',', found name
docs/io/io.rst:72:17: SyntaxError: Expected ',', found name
docs/io/io.rst:72:21: SyntaxError: Expected ',', found name
docs/io/io.rst:72:37: SyntaxError: Expected ',', found name
docs/io/io.rst:72:50: SyntaxError: Expected ',', found name
docs/io/io.rst:72:57: SyntaxError: Expected ',', found name
docs/io/io.rst:72:64: SyntaxError: Expected ',', found name
docs/io/io.rst:74:1: SyntaxError: Expected an indented block after `with` statement
docs/io/io.rst:74:2: SyntaxError: Expected a statement
docs/io/io.rst:74:4: SyntaxError: Invalid annotated assignment target
docs/io/io.rst:74:15: SyntaxError: Expected an expression
docs/io/io.rst:76:1: SyntaxError: Unexpected indentation
docs/io/io.rst:93:1: SyntaxError: Expected a statement
Found 151 errors.
[*] 1 fixable with the `--fix` option.

@sundanc sundanc changed the title Added S92Reader for Shull & Van Steenberg photoionization data - GSoC 2025 [GSOC]-Added S92Reader for Shull & Van Steenberg photoionization data Mar 31, 2025
@sundanc
Copy link
Author

sundanc commented Mar 31, 2025

@andrewfullard @jvshields

@wkerzendorf wkerzendorf requested a review from Copilot April 1, 2025 13:58
Copy link

@Copilot 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 adds support for reading and processing Shull & Van Steenberg (1992) photoionization data into TARDIS by introducing a new S92Reader class and integrating it into the existing output routines. Key changes include:

  • A new module (carsus/io/s92) with S92Reader to parse and process s92.201 data files.
  • Comprehensive test cases for verifying file parsing, cross-section calculations, and integration.
  • Modifications in the output module (carsus/io/output/base.py) to incorporate S92Reader results into the photoionization data stored in HDF5.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
carsus/io/s92/tests/test_s92.py Added tests for S92Reader functionality including file parsing and cross-section checks.
carsus/io/s92/base.py Implements the S92Reader class with file download, parsing, and cross-section calculation logic.
carsus/io/s92/init.py Exposes S92Reader and S92_FTP_URL for external use.
carsus/io/output/base.py Updated integration in the photoionization data creation workflow using S92Reader.
Files not reviewed (1)
  • docs/io/io.rst: Language not supported

energies = np.array([10.0, 15.0, 20.0, 30.0])
sigmas = reader.calculate_cross_section(energies, 1, 0)
assert len(sigmas) == len(energies)
assert sigmas[0] == 0.0 # Below threshold
Copy link
Preview

Copilot AI Apr 1, 2025

Choose a reason for hiding this comment

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

[nitpick] Direct equality comparisons for floating-point values can be unreliable; consider using np.isclose() to compare values with a tolerance.

Suggested change
assert sigmas[0] == 0.0 # Below threshold
assert np.isclose(sigmas[0], 0.0) # Below threshold

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@wkerzendorf
Copy link
Member

Currently tests fail or were cancelled

@sundanc
Copy link
Author

sundanc commented Apr 14, 2025

Currently tests fail or were cancelled

I caught up by the exams, and couldn't fix the issues. I will try to work on it again as fast as I can @wkerzendorf

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.

2 participants