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

Fix testeff again #1177

Merged
merged 6 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions simtools/applications/validate_camera_efficiency.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,15 @@ def _parse(label):
required=False,
)
config.parser.add_argument(
"--apply_correction_to_nsb_spectrum",
"--skip_correction_to_nsb_spectrum",
help=(
"Apply a correction to the NSB spectrum to account for the "
"difference between the altitude used in the reference B&E spectrum and "
"the observation level at the CTAO sites."
"This correction is done internally in sim_telarray and is on by default."
),
type=bool,
default=True,
required=False,
action="store_true",
)
_args_dict, _db_config = config.initialize(
db_config=True,
Expand Down
8 changes: 6 additions & 2 deletions simtools/camera_efficiency.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ def simulate(self):
file_log=self._file["log"],
label=self.label,
nsb_spectrum=self.config["nsb_spectrum"],
apply_correction_to_nsb_spectrum=self.config.get(
"apply_correction_to_nsb_spectrum", True
skip_correction_to_nsb_spectrum=self.config.get(
"skip_correction_to_nsb_spectrum", False
),
)
simtel.run(test=self.test)
Expand All @@ -180,6 +180,10 @@ def export_model_files(self):
"""Export model and config files to the output directory."""
self.telescope_model.export_config_file()
self.telescope_model.export_model_files()
if not self.config.get("skip_correction_to_nsb_spectrum", False):
self.telescope_model.export_nsb_spectrum_to_telescope_altitude_correction_file(
model_directory=self.telescope_model.config_file_directory
)

def analyze(self, export=True, force=False):
"""
Expand Down
26 changes: 26 additions & 0 deletions simtools/model/model_parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -570,3 +570,29 @@ def _load_simtel_config_writer(self):
model_version=self.model_version,
label=self.label,
)

def export_nsb_spectrum_to_telescope_altitude_correction_file(self, model_directory):
"""
Export the NSB spectrum to the telescope altitude correction file.

This method is needed because testeff corrects the NSB spectrum from the original altitude
used in the Benn & Ellison model to the telescope altitude.
This is done internally in testeff, but the NSB spectrum is not written out to the model
directory. This method allows to export it explicitly.

Parameters
----------
model_directory: Path
Model directory to export the file to.
"""
self.db.export_model_files(
{
"nsb_spectrum_at_2200m": {
"value": self._simulation_config_parameters["simtel"][
"correct_nsb_spectrum_to_telescope_altitude"
]["value"],
"file": True,
}
},
model_directory,
)
6 changes: 3 additions & 3 deletions simtools/simtel/simulator_camera_efficiency.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __init__(
file_log=None,
zenith_angle=None,
nsb_spectrum=None,
apply_correction_to_nsb_spectrum=True,
skip_correction_to_nsb_spectrum=False,
):
"""Initialize SimtelRunner."""
self._logger = logging.getLogger(__name__)
Expand All @@ -54,7 +54,7 @@ def __init__(
self._file_log = file_log
self.zenith_angle = zenith_angle
self.nsb_spectrum = nsb_spectrum
self.apply_correction_to_nsb_spectrum = apply_correction_to_nsb_spectrum
self.skip_correction_to_nsb_spectrum = skip_correction_to_nsb_spectrum

@property
def nsb_spectrum(self):
Expand Down Expand Up @@ -122,7 +122,7 @@ def _make_run_command(
)

command = str(self._simtel_path.joinpath("sim_telarray/testeff"))
if not self.apply_correction_to_nsb_spectrum:
if self.skip_correction_to_nsb_spectrum:
command += " -nc" # Do not apply correction to original altitude where B&E was derived
command += " -I" # Clear the fall-back configuration directories
command += f" -I{self._telescope_model.config_file_directory}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 90 and 1000 (very loose requirement)
photons: [90, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 70 and 1000 (very loose requirement)
photons: [70, 1000]
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ CTA_SIMPIPE:
# should be between 20 and 1000 (very loose requirement)
pe_sum: [20, 1000]
# The mean number of photons per telescope after atmospheric absorption and QE
# should be between 100 and 1000 (very loose requirement)
photons: [100, 1000]
# should be between 70 and 1000 (very loose requirement)
photons: [70, 1000]
6 changes: 6 additions & 0 deletions tests/integration_tests/test_applications_from_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,12 @@ def skip_camera_efficiency(config):
"due to a limitation of the old testeff not allowing to specify "
"the include directory. Please update your sim_telarray tarball."
)
full_test_name = f"{config['APPLICATION']}_{config['TEST_NAME']}"
if "simtools-validate-camera-efficiency_SSTS" == full_test_name:
pytest.skip(
"The test simtools-validate-camera-efficiency_SSTS is skipped "
"since the fake SST mirrors are not yet implemented (#1155)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the expression 'fake'. What does it mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to provide a list of mirrors in the "standard style" of sim_telarray for SST. KB called it fake_astri_mirrorpos.dat, so I did the same here. Either way, this comment will disappear when #1155 is fixed.

)


def new_testeff_version():
Expand Down
1 change: 1 addition & 0 deletions tests/unit_tests/db/test_db_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def test_reading_db_lst_without_simulation_repo(db, model_version):
db_copy.mongo_db_config["db_simulation_model_url"] = None
pars = db.get_model_parameters("North", "LSTN-01", model_version, collection="telescopes")
assert pars["parabolic_dish"]["value"] == 1
assert pars["camera_pixels"]["value"] == 1855


def test_reading_db_lst(db, model_version):
Expand Down
25 changes: 24 additions & 1 deletion tests/unit_tests/model/test_model_parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def test_get_simtel_parameters(telescope_model_lst):


def test_change_parameter(telescope_model_lst):
tel_model = telescope_model_lst
tel_model = copy.deepcopy(telescope_model_lst)

logger.info(f"Old camera_pixels:{tel_model.get_parameter_value('camera_pixels')}")
tel_model.change_parameter("camera_pixels", 9999)
Expand Down Expand Up @@ -340,3 +340,26 @@ def test_get_config_file(telescope_model_lst, mocker):
telescope_copy._is_config_file_up_to_date = False
telescope_copy.get_config_file(no_export=True)
not mock_export.assert_called_once()


def test_export_nsb_spectrum_to_telescope_altitude_correction_file(telescope_model_lst, mocker):
model_directory = Path("test_model_directory")
telescope_copy = copy.deepcopy(telescope_model_lst)

mock_db_export = mocker.patch.object(DatabaseHandler, "export_model_files")
mock_simulation_config_parameters = {
"simtel": {"correct_nsb_spectrum_to_telescope_altitude": {"value": "test_value"}}
}
telescope_copy._simulation_config_parameters = mock_simulation_config_parameters

telescope_copy.export_nsb_spectrum_to_telescope_altitude_correction_file(model_directory)

mock_db_export.assert_called_once_with(
{
"nsb_spectrum_at_2200m": {
"value": "test_value",
"file": True,
}
},
model_directory,
)
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_make_run_command_with_nsb_spectrum(simulator_camera_efficiency, expecte
def test_make_run_command_without_altitude_correction(
simulator_camera_efficiency, expected_command, benn_ellison_spectrum_file_name
):
simulator_camera_efficiency.apply_correction_to_nsb_spectrum = False
simulator_camera_efficiency.skip_correction_to_nsb_spectrum = True
command = simulator_camera_efficiency._make_run_command()

for item in expected_command:
Expand Down