Skip to content

Conversation

@GernotMaier
Copy link
Contributor

@GernotMaier GernotMaier commented Jan 23, 2025

Add functionality to compare simtools-generated configuration files for sim_telarray with reference file:

  • line-by-line string comparison (this might lead to issue with comparison of floats; for now it seems to work)
  • reference files are available in tests/resources/sim_telarray_configurations/
  • implemented for the integration tests simulate_prod_gamma_40_deg_North_check_output.yml and simulate_prod_gamma_40_deg_South_check_output.yml (see the keyword TEST_SIMTEL_CFG_FILES)
  • changed simulate_prod_gamma_40_deg_South_check_output.yml to beta layout to allow to test also LSTS

Some minor fine tuning in the file comparison is required as lines like label and 'config_version' (as they change for different integration tests).

Note: there is no need to review the reference configuration files in tests/resources/sim_telarray_configurations.

Important!! Model parameters in the reference files are not entirely correct but are 'as exported' from the current version of the model parameter database. #1326 provides corrections so that the model parameters will be correct.

@GernotMaier GernotMaier self-assigned this Jan 23, 2025
@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

@GernotMaier GernotMaier marked this pull request as ready for review January 27, 2025 08:25
@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

@ctao-dpps-sonarqube

This comment has been minimized.

Copy link
Collaborator

@EshitaJoshi EshitaJoshi left a comment

Choose a reason for hiding this comment

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

Code looks good. I have only skimmed at the config files and not reviewed them, as you said. Two questions - why are we not including units information for the parameters? And, how are these config files going to be updated when the model parameters are updated in the future? Will that have to be done manually?

@GernotMaier
Copy link
Contributor Author

Code looks good. I have only skimmed at the config files and not reviewed them, as you said. Two questions - why are we not including units information for the parameters?

These are configuration files used by sim_telarray. There are 'expected' units for sim_telarray which we have written up in the schema files. All value sin sim_telarray needs to be given in these expected units.

And, how are these config files going to be updated when the model parameters are updated in the future? Will that have to be done manually?

We don't expect any updates to these reference files (until we see bugs), as we don't expect any future model versions to be distributed with sim_telarray (they will be distributed using simtools and the Simulation-Models database). So the tests included in this PR are ensuring that our base models (prod5/prod6) are correct (plus all the machinery to write these base models in simtools)

@ctao-dpps-sonarqube
Copy link

Passed

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 1 Code Smell

Coverage and Duplications

  • Coverage 100.00% Coverage (93.70% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: gammasim_simtools_AY_ssha9WiFxsX-2oy_w

View in SonarQube

@EshitaJoshi EshitaJoshi self-requested a review January 27, 2025 15:41
Copy link
Collaborator

@EshitaJoshi EshitaJoshi left a comment

Choose a reason for hiding this comment

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

Code is good, thanks for the PR!

@GernotMaier
Copy link
Contributor Author

@EshitaJoshi - thank you!

@GernotMaier GernotMaier merged commit 02412a5 into main Jan 27, 2025
14 checks passed
@GernotMaier GernotMaier deleted the test-simtel-cfg-files branch January 27, 2025 16:55
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