Skip to content

Conversation

@steven-murray
Copy link
Member

Add a simpler lightcone function for 21cmFAST output structs in files.

Copy link

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 introduces a new function construct_lightcone_from_filelist that allows creating lightcones directly from a list of 21cmFAST output struct files, providing a simpler alternative to the existing cache-based approach.

  • Adds construct_lightcone_from_filelist function for direct file-based lightcone construction
  • Updates existing tests to use more realistic redshift ranges and dynamic shape assertions
  • Adds comprehensive test coverage for the new file-based lightcone construction

Reviewed Changes

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

File Description
src/tuesday/simulators/py21cmfast/lightcones.py Implements the new construct_lightcone_from_filelist function and updates imports
tests/test_simulators/test_py21cmfast/test_lightcones.py Adds test coverage for the new function and improves existing tests

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

Comment on lines +126 to +128
global_quantities
A list of global quantities to extract. These must exist in the files
given.
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

The docstring references a global_quantities parameter that doesn't exist in the function signature. This parameter should be removed from the documentation or added to the function.

Suggested change
global_quantities
A list of global quantities to extract. These must exist in the files
given.

Copilot uses AI. Check for mistakes.
)

prev_box = None
for box in boxes:
Copy link

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

This comment indicates a temporary workaround that may become obsolete. Consider adding a TODO comment or tracking issue to revisit this implementation when newer versions of 21cmFAST are available.

Suggested change
for box in boxes:
for box in boxes:
# TODO: Remove this hack when upgrading to a newer version of 21cmFAST that natively supports this.

Copilot uses AI. Check for mistakes.
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