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

Verification of control files #119

Open
sivogel opened this issue Sep 21, 2023 · 2 comments
Open

Verification of control files #119

sivogel opened this issue Sep 21, 2023 · 2 comments
Assignees

Comments

@sivogel
Copy link
Member

sivogel commented Sep 21, 2023

The intended behaviour of control files is as follows: If parameters are stored in a control file and have not been previously set, the parameter is automatically set to "?" in the control files. Attempting to read these control files will result in a syntax error, reminding the user to set these parameters.
But: no error will be thrown when reading control files if the parameter is set to None (in the control files this parameter is stored with empty brackets). This happens when, for example, an empty list or dictionary is assigned. The control files can be loaded without error, even if this parameterisation would result in a non-executable model.
Setting parameters of type Parameter or SeasonalParameter to empty lists or dictionaries in control files will result in arrays of zeros which could lead to unintentional parameterisation, perhaps we can add some safety mechanisms here.
Even more critical is the assignment of empty lists or dictionaries in control files to e.g. interpolator parameters, the simulation will abort without an ErrorCode.
Using model.parameters.verify() could help to catch this error for e.g. interpolator parameters like waterlevel2flooddischarge. Therefore, automatically applying verify() to the model parameters, after reading models from control files would be a good improvement.
This parameter verification issue is closely related to issue #55

@tyralla
Copy link
Member

tyralla commented Sep 21, 2023

Thanks for the report, Simone.

Could you please give short examples for "Setting parameters of type Parameter or SeasonalParameter to empty lists or dictionaries in control files will result in arrays of zeros"? (both for Parameter and SeasonalParameter). And also for the "interpolator parameters" (does this problem occur both for SimpleInterpolator and SeasonalInterpolator subtypes?)

For consistency, we could do the verification while writing the control files. This would also be more efficient, because reading control files happens more often than writing them.

@sivogel
Copy link
Member Author

sivogel commented Sep 21, 2023

Perhaps I have chosen an inaccurate formulation saying "Setting parameters of type Parameter or SeasonalParameter to empty lists or dictionaries in control files will result in arrays of zeros", so let me add the following explanantion:
In my case, I wanted to associate the parameters with a dictionary with toy keys that I automatically generated from an input database. Since the underlying database was missing information it happened that I generated empty dictionaries. Simplified I did the following:

d_dict = {} 
element.model.parameters.control.neardischargeminimumthreshold(**d_dict)

This gave no error and was saved in the control file as follows:

neardischargeminimumthreshold()

This makes pretty much sense with the input I gave, but is not a valid parametrization and therefore should lead to an error.

The SeasonalInterpolator parameters show the same behaviour

@tyralla tyralla mentioned this issue Dec 4, 2023
30 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants