Skip to content

Conversation

@GernotMaier
Copy link
Contributor

Allow to test reference configuration files for prod5/prod6 against the original sim_telarray
configuration files.

Uses prod5/prod6 configurations uploaded to Zenodo (plus to files related to prod5-Alpha, which unfortunately are not part of the Zenodo archive)

@GernotMaier GernotMaier self-assigned this Jan 26, 2025
@GernotMaier GernotMaier requested a review from Copilot January 29, 2025 10:54
@GernotMaier
Copy link
Contributor Author

@orelgueta - I think it would be good if you browse through the two test scripts in src/simulation_model to make sure that the testing of the nominal prod5/prod6 model is reasonable.

I don't think we want to enforce the strict coding / reviewing rules of simtools to the scripts here.

@GernotMaier GernotMaier requested a review from orelgueta January 29, 2025 10:56
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.

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

Files not reviewed (3)
  • src/simulation_model/cfg/CTA-PROD5-Paranal-Alpha.cfg: Language not supported
  • src/simulation_model/cfg/array_trigger_prod5_paranal_alpha.dat: Language not supported
  • .github/workflows/CI-linter.yml: Evaluated as low risk
Comments suppressed due to low confidence (6)

src/simulation_model/download_configuration_from_zenodo.py:33

  • The comment is unclear. Suggestion: 'Suppress RuntimeWarning; this warning is expected to be resolved in Python 3.12.'
warnings; should go away with python 3.12

src/simulation_model/test_reference_simtel_configuration.py:235

  • Mismatch in the number of telescopes between sim_telarray and simtools configuration files.
raise ValueError("Number of telescopes in sim_telarray and simtools configuration files do not match.")

src/simulation_model/test_reference_simtel_configuration.py:256

  • Replace print statements with logger.error or logger.info as appropriate.
print("Lines differ:")

src/simulation_model/test_reference_simtel_configuration.py:257

  • Replace print statements with logger.error or logger.info as appropriate.
print("\t SIMTEL  :", simtel_line)

src/simulation_model/test_reference_simtel_configuration.py:5

  • Correct the spacing around the slash.
simtools-derived model parameters / configuration files using the simulation model database with those obtained using sim_telarray.

src/simulation_model/test_data_tables.py:51

  • [nitpick] The error message 'Files {file} and {simtel_path} differ' is not very specific. Consider including the line numbers where the files differ.
logger.error(f"Files {file} and {simtel_path} differ")

if simtel_line != simtools_line:
print("Lines differ:")
print("\t SIMTEL :", simtel_line)
print("\t SIMTOOLS:", simtools_line)
Copy link

Copilot AI Jan 29, 2025

Choose a reason for hiding this comment

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

Replace print statements with logger.error or logger.info as appropriate.

Suggested change
print("\t SIMTOOLS:", simtools_line)
logger.error('Lines differ:\n\t SIMTEL : %s\n\t SIMTOOLS: %s', simtel_line, simtools_line)

Copilot uses AI. Check for mistakes.
simtel_par = [line for line in simtel_cfg if par.upper() in line.upper()]
simtools_par = [line for line in simtools_cfg if par.upper() in line.upper()]
if simtel_par != simtools_par:
print("Site parameter differ:")
Copy link

Copilot AI Jan 29, 2025

Choose a reason for hiding this comment

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

Replace print statements with logger.error or logger.info as appropriate.

Suggested change
print("Site parameter differ:")
logger.error('Lines differ:')

Copilot uses AI. Check for mistakes.
simtools_par = [line for line in simtools_cfg if par.upper() in line.upper()]
if simtel_par != simtools_par:
print("Site parameter differ:")
print("\t SIMTEL :", simtel_par)
Copy link

Copilot AI Jan 29, 2025

Choose a reason for hiding this comment

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

Replace print statements with logger.error or logger.info as appropriate.

Suggested change
print("\t SIMTEL :", simtel_par)
logger.error('Site parameter differ:')

Copilot uses AI. Check for mistakes.
if simtel_par != simtools_par:
print("Site parameter differ:")
print("\t SIMTEL :", simtel_par)
print("\t SIMTOOLS:", simtools_par)
Copy link

Copilot AI Jan 29, 2025

Choose a reason for hiding this comment

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

Replace print statements with logger.error or logger.info as appropriate.

Suggested change
print("\t SIMTOOLS:", simtools_par)
logger.error('Site parameter differ:')

Copilot uses AI. Check for mistakes.
Copy link

@orelgueta orelgueta left a comment

Choose a reason for hiding this comment

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

Did not do a thorough review, but it all looks good. Thanks!

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.

3 participants