Skip to content
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

Introduce MSTx-NectarCam and MSTx-FlashCam #1362

Merged
merged 40 commits into from
Feb 24, 2025
Merged

Conversation

GernotMaier
Copy link
Contributor

@GernotMaier GernotMaier commented Feb 11, 2025

This PR introduces functionality to handle design models located on several sites. This mostly required changes in the validation modules:

  • allow model parameter key 'site' to be a list of sites (e.g., ['North', 'South'] instead of only one of them); requires introduction of a new version of the model_parameter meta schema (new version is 0.3.0)
  • replace MSTN-design by MSTx-NectarCam and MSTS-design by MSTx-FlashCam. This requires to introduce in the list of array elements the following new entry:
  MSTx:
    collection: "telescopes"
    observatory: "CTAO"
    site: ["North", "South"]
    design_types: ["design", "test", "FlashCam", "NectarCam"]
    description: "Mid-sized telescope"
  • added a new function to get the check the allowed design types: names.array_element_design_types
  • note that the test for the design type is now case sensitive (as the mongoDB query is case sensitive)
  • improve inconsistent testing of parameter versions and file names for parameter (testing for typically errors found during setup of the DB); this required then as a follow up to rename the model parameter json files in tests/resources to be consistent with our standards
  • corresponding issue and merge request

Requires a simulation model database >0.3.0

@GernotMaier GernotMaier self-assigned this Feb 11, 2025
@GernotMaier GernotMaier marked this pull request as ready for review February 13, 2025 08:56
@GernotMaier GernotMaier requested a review from Copilot February 13, 2025 09:00

Choose a reason for hiding this comment

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

Copilot reviewed 30 out of 45 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • src/simtools/layout/array_layout.py: Evaluated as low risk
  • src/simtools/visualization/legend_handlers.py: Evaluated as low risk
  • tests/integration_tests/config/derive_mirror_rnda_psf_random_flen.yml: Evaluated as low risk
  • tests/conftest.py: Evaluated as low risk
  • tests/integration_tests/config/derive_mirror_rnda_psf_no_tuning.yml: Evaluated as low risk
  • tests/integration_tests/config/validate_camera_efficiency_msts.yml: Evaluated as low risk
  • tests/integration_tests/config/submit_model_parameter_from_external_submit_mirror_list.yml: Evaluated as low risk
  • tests/integration_tests/config/db_get_parameter_from_db_telescope_parameter_version.yml: Evaluated as low risk
  • tests/integration_tests/config/validate_camera_efficiency_mstn.yml: Evaluated as low risk
  • tests/integration_tests/config/derive_mirror_rnda_psf_measurement.yml: Evaluated as low risk
  • src/simtools/schemas/array_elements.yml: Evaluated as low risk
  • src/simtools/applications/validate_file_using_schema.py: Evaluated as low risk
  • tests/integration_tests/config/db_get_parameter_from_db_array_element_position_ground.yml: Evaluated as low risk
  • src/simtools/db/db_handler.py: Evaluated as low risk
  • src/simtools/utils/names.py: Evaluated as low risk

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

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.

Thanks for the PR! I have a few minor comments.

@@ -3,9 +3,9 @@ CTA_SIMPIPE:
TEST_NAME: fov-run
CONFIGURATION:
SITE: North
TELESCOPE: MSTN-design
TELESCOPE: MSTx-NectarCam
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also be validating the fov for FlashCam?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really necessary, we are validating the 'validate_camera_fov' application and not the the FOV of all telescope types. But I see that this is not consistent with the earlier validate_camera_efficiency tests.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

Passed

Analysis Details

0 Issues

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

Coverage and Duplications

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

Project ID: gammasim_simtools_AY_ssha9WiFxsX-2oy_w

View in SonarQube

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.

Thanks for the PR!

@GernotMaier
Copy link
Contributor Author

Thanks @EshitaJoshi for reviewing this large PR!

@GernotMaier GernotMaier merged commit 7af325d into main Feb 24, 2025
15 checks passed
@GernotMaier GernotMaier deleted the flash-and-nectarcam branch February 24, 2025 12:59
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