Refactor db_handler and model_parameter modules; add possibility to change parameters from command line #1850

GernotMaier · 2025-10-20T14:00:44Z

Adds functionality and addresses a couple of issues regarding model parameters and database handling. A bit of a big pull request, but things are somehow interwoven and sometimes difficult to separate.

Change model parameter from command line

Closes #1795

Allow to change model parameter values from the command line using the same format as used in the 'info.yml' files in the simulation models, e.g.:

changes:
  LSTN-01:
    nsb_pixel_rate:
      version: "3.0.0"
      value: 1.17456
      unit: GHz

changes the values for LSTN-01.

See the new test file tests/resources/info_test_model_parameter_changes.yml for a complete example.

Simplified changed from simtools modules

The original way of changing parameters ("par_name": value) is kept for backwards compatibility with existing code (applied only to TelescopeModel or SiteModel)

Refactoring of db_handler class

db_handler is too big and this PR separates code related specifically to the mongoDB into simtools.db.mongo_db.

Closes #1769

Add checks for changes in metadata

Add the functionality for the integration tests to test the metdata of the sim_telarray output files. E.g. given in the test configuration file the following block

integration_tests:
       - expected_simtel_metadata:
        site_config_name: South
        1:
          effective_focal_length: '2923.7 0 0 2 0'
        2:
          effective_focal_length: '2923.7 0 0 0 0'
        4:
          effective_focal_length: '1644.51 0 0 0 0'
        5:
          effective_focal_length: '315.191 0 0 0 0'
        38:
          effective_focal_length: '215.191 0 0 0 0'
      file: gamma_run000001_za40deg_azm180deg_South_beta_6.0_check_output.simtel.zst
      path_descriptor: pack_for_grid_register

Will query the metadata for the given values and lead to a failure if the comparison fails.
Note that the numbers correspond to the sim_telarray numbering of telescopes used in the metadata. I know this is not ideal, but probably fine as a start.

General cleanup

Parsed carefully through the code and improved / simplified anything obvious. Probably the most annoying (for reviewer) changes throughout the code is mongo_db_config to db_config (there is no need to embed the actual database technology into the code).

Main integration test

Modifying parameters and testing the modifications has been added to this integration test:

pytest --no-cov -n auto --model_version=6.0 tests/integration_tests/test_applications_from_config.py::test_applications_from_config[simtools-simulate-prod_gamma_40_deg_south_check_output]

The modification file is in tests/resources/info_test_model_parameter_changes.yml

Possible issues / notes

Parameter values might be changed now in the simtools.models.model_parameter module. This means we cannot rely on the (quite efficient) structure from db_handler where parameters for a given telescope types are not duplicated (if all or some of them are identical). There is now a deepcopy in simtools.model.model_parameters._load_parameters_from_db() - otherwise very hard to change parameter for a given telescope, but not for all. I am pretty sure this introduces not a performance or memory issue (unless we start simulate a huge number of telescopes).

missing: remove parameters which are labeled deprecated in the file given to overwrite parameters (needed?)
missing: add new parameters (parameters which are not yet part of the model parameter dict for the given array model) listed in the file given to overwrite parameters (needed?)

s

Copilot

Pull Request Overview

This pull request refactors the database handling and model parameter modules, primarily separating MongoDB-specific code into a dedicated module and adding functionality to overwrite model parameters from the command line. The changes enable users to modify simulation model parameters at runtime using YAML files following the model repository format, while maintaining backward compatibility with existing code.

Key Changes:

Separated MongoDB-specific database operations into a new simtools.db.mongo_db module
Renamed mongo_db_config to db_config throughout the codebase to decouple from specific database technology
Added overwrite_model_parameters parameter to model classes and command-line interface
Refactored parameter modification methods (renamed change_parameter to overwrite_model_parameter, etc.)
Added functionality to validate sim_telarray metadata in integration tests

Reviewed Changes

Copilot reviewed 71 out of 71 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tests/unit_tests/model/test_model_parameter.py`	Updated tests for renamed parameter modification methods and removed obsolete tests
`tests/unit_tests/db/test_db_handler.py`	Refactored tests to use new MongoDB handler delegation pattern
`tests/unit_tests/db/test_mongo_db.py`	New comprehensive test suite for MongoDB-specific operations
`tests/conftest.py`	Updated fixtures to use `db_config` parameter and changed default model version
`src/simtools/db/mongo_db.py`	New module containing MongoDB-specific database operations
`src/simtools/db/db_handler.py`	Refactored to delegate MongoDB operations to the new handler
`src/simtools/model/model_parameter.py`	Added parameter overwriting functionality and renamed methods
`src/simtools/testing/assertions.py`	Added sim_telarray metadata validation support
`src/simtools/configuration/commandline_parser.py`	Added `--overwrite_model_parameters` command-line argument
Multiple application files	Updated to use `db_config` instead of `mongo_db_config`

src/simtools/utils/value_conversion.py

src/simtools/data_model/schema.py

src/simtools/model/model_parameter.py

tobiaskleiner

Added few minor comments, thanks @GernotMaier for the PR.

tobiaskleiner · 2025-10-23T12:35:04Z

src/simtools/db/mongo_db.py

+
+    db_client: MongoClient | None = None
+    _lock = Lock()
+


Would it make senser to add a close connection here (close_connection()) to close the client?

The MongoClient is thread-safe and maintains its own connection pool . For normal process exit, sockets and threads are cleaned up; no need to call close() just to avoid leaks. So I think no close() is need.

tobiaskleiner · 2025-10-23T12:36:02Z

src/simtools/db/mongo_db.py

+    def __init__(self, db_config=None):
+        """Initialize the MongoDBHandler class."""
+        self._logger = logging.getLogger(__name__)
+        self.db_config = self._validate_db_config(db_config)


You could call self.validate_db_config(db_config) directly here?

tobiaskleiner · 2025-10-23T12:36:45Z

src/simtools/db/mongo_db.py

+        datetime.datetime
+            The generation time of the document's ObjectId.
+        """
+        return ObjectId(document["_id"]).generation_time


Do we need a check here that id exists?

Yes and no. If _id doesn’t exist, than the DB is messed up and we have a larger problem. I think the KeyError issued in this cases will be clear enough.

tobiaskleiner · 2025-10-23T12:46:39Z

src/simtools/db/mongo_db.py

+            # _id is a public attribute in GridFS GridOut objects
+            # pylint: disable=protected-access
+            return file_system.find_one({"filename": kwargs["filename"]})._id
+


Should we add a check that the file exists here ?

For filename: all DB entries are extensively validated against their schema at the filling stage. I think we can safely assume that this key exists.

tobiaskleiner · 2025-10-23T12:49:48Z

src/simtools/db/mongo_db.py

+        """
+        if self.db_config:
+            db_server = self.db_config["db_server"]
+            domain_pattern = r"^([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$"


There is also ipaddress.ip_address() to check for IP adress. Not sure regex would work for IPv4 adress?

We are testing domain names like ‘"cta-simpipe-protodb.zeuthen.desy.de”’ vs localhost, not IP addresses. Is this still an issue?

tobiaskleiner · 2025-10-23T12:54:17Z

src/simtools/db/mongo_db.py

+        -------
+        list
+            List of collection names
+        """


I guess that the cache is not renewed doesnt pose a problem here. Or do we need a refresh or discard method for any case?

Not sure I get this one - the scenario would be that we add collections between the first call and a later one? I don't think we do this at all.

ctao-sonarqube · 2025-10-24T06:53:48Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
99.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

GernotMaier · 2025-10-24T06:54:56Z

@tobiaskleiner - thanks for the careful review. Please have a look and let me know if this is ok.

tobiaskleiner · 2025-10-24T11:03:42Z

Thanks @GernotMaier, yes all good now.

tests/resources/info_test_model_parameter_changes.yml

GernotMaier added 10 commits October 19, 2025 20:57

cleanup db config

0598592

refactor db_handler

e6699ca

Merge branch 'main' into modify-model-parameters

02bf6e5

tests

2662abb

naming

49808cc

testing

5743ffd

changes

d248769

changes

e656ab7

testt push

afc28f2

s

tests

ab7b9aa

GernotMaier self-assigned this Oct 20, 2025

GernotMaier added 8 commits October 20, 2025 18:28

Merge branch 'main' into modify-model-parameters

d3f6ad7

tests

0ece132

fix attributeerror

ce7dbd1

Merge branch 'main' into modify-model-parameters

c302bda

test file for changes through command line interface

fcb6fdd

add assertion of metadata

f945402

integration test

6417479

details

5fc6414

GernotMaier requested a review from Copilot October 22, 2025 10:52

Copilot AI reviewed Oct 22, 2025

View reviewed changes

src/simtools/utils/value_conversion.py Outdated Show resolved Hide resolved

src/simtools/data_model/schema.py Outdated Show resolved Hide resolved

src/simtools/model/model_parameter.py Outdated Show resolved Hide resolved

GernotMaier added 2 commits October 22, 2025 13:03

copilot

fd336ce

Merge branch 'main' into modify-model-parameters

22bbd7a

GernotMaier marked this pull request as ready for review October 23, 2025 08:18

GernotMaier requested a review from tobiaskleiner October 23, 2025 08:18

GernotMaier added 2 commits October 23, 2025 10:39

changelog

ff556e1

docs

547350c

tobiaskleiner requested changes Oct 23, 2025

View reviewed changes

rev

a394505

tobiaskleiner approved these changes Oct 24, 2025

View reviewed changes

GernotMaier merged commit 037d776 into main Oct 24, 2025
10 checks passed

GernotMaier deleted the modify-model-parameters branch October 24, 2025 12:06

Copilot AI mentioned this pull request Oct 28, 2025

v0.24.0-rc #1869

Merged

orelgueta reviewed Dec 17, 2025

View reviewed changes

tests/resources/info_test_model_parameter_changes.yml Show resolved Hide resolved

GernotMaier mentioned this pull request Jan 30, 2026

Overwrite model parameters: fix warning #2003

Merged

Refactor db_handler and model_parameter modules; add possibility to change parameters from command line #1850

Refactor db_handler and model_parameter modules; add possibility to change parameters from command line #1850

Uh oh!

Conversation

GernotMaier commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change model parameter from command line

Simplified changed from simtools modules

Refactoring of db_handler class

Add checks for changes in metadata

General cleanup

Main integration test

Possible issues / notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tobiaskleiner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ctao-sonarqube bot commented Oct 24, 2025

Quality Gate passed

Uh oh!

GernotMaier commented Oct 24, 2025

Uh oh!

tobiaskleiner commented Oct 24, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GernotMaier commented Oct 20, 2025 •

edited

Loading