Add application to import model parameters from simtel configuration file #824
Conversation
GernotMaier
changed the base branch from
new-simulation-model
to
dev-simulation-model-elements-naming
simtools/applications/convert_all_model_parameters_from_simtel.py
Outdated
Show resolved
Hide resolved
simtools/applications/convert_all_model_parameters_from_simtel.py
Outdated
Show resolved
Hide resolved
| or len(simtel_config_reader.parameter_dict) == 0 | ||
| ): | ||
| _parameters_not_in_simtel.append(_parameter) | ||
| logger.error("Parameter not found in sim_telarray configuration file.") |
There was a problem hiding this comment.
Should we maybe add a reminder to raise an error here in the future?
There was a problem hiding this comment.
I am actually more included to 'downgrade this to a logger.info statement - it is not really an error, as the sim_telarray configuration includes also parameters we will never ever use (as they are either for outdated instruments, or for very special use cases). So this is expected, and I change it to an info.
There was a problem hiding this comment.
Okey this is good then.
| f"Data column '{column_name}' not found in reference column definition" | ||
| ) | ||
| raise exc | ||
| except TypeError: |
There was a problem hiding this comment.
Not entirely sure how we can get this exception. Should we not also raise something here?
| Returns | ||
| ------- | ||
| object, int | ||
| Values extracted from column. Of object is a list of array, return length of array. |
There was a problem hiding this comment.
Thanks @tobiaskleiner for the review!
I tried to address your points, see below (and new commits). Let me know if this looks reasonable.
simtools/applications/convert_all_model_parameters_from_simtel.py
Outdated
Show resolved
Hide resolved
simtools/applications/convert_all_model_parameters_from_simtel.py
Outdated
Show resolved
Hide resolved
| or len(simtel_config_reader.parameter_dict) == 0 | ||
| ): | ||
| _parameters_not_in_simtel.append(_parameter) | ||
| logger.error("Parameter not found in sim_telarray configuration file.") |
There was a problem hiding this comment.
I am actually more included to 'downgrade this to a logger.info statement - it is not really an error, as the sim_telarray configuration includes also parameters we will never ever use (as they are either for outdated instruments, or for very special use cases). So this is expected, and I change it to an info.
|
Thanks @GernotMaier for adressing the comments. It's ready for merging now. |
GernotMaier
merged commit 1a35021
into
dev-simulation-model-elements-naming
This PR
The full model parameters used by sim_telarray can be exported using e.g.
There is an example for testing in tests/resources/simtel_config_test_la_palma.cfg and two integration tests are available. The two applications
convert_all_model_parameter_from_simtel.pyandconvert_model_parameter_from_simtel.pyare quite rich in printout - this is needed during the development phase to make sure that there is no incompatibility between schemas and sim_telarray.For the data validation:
Fixed this in the process of this pull request and improved the tests significantly.
validate_file_using_schemawas very confusing and did not always work as expected. Cleaned this up to have now clearly separated on how the validation works depending on the data type (metadata, schema, data). Adjusted and renamed the integration tests accordingly.There is no need to review any schema files added or modified by this PR. This has been or will be review as part of the model parameter repository.