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

Consistency in relation between use_* fields and corresponding tables #129

Open
1 of 2 tasks
leendertvanwolfswinkel opened this issue Oct 23, 2024 · 6 comments
Open
1 of 2 tasks
Assignees

Comments

@leendertvanwolfswinkel
Copy link

leendertvanwolfswinkel commented Oct 23, 2024

This applies to the following settings:

Model settings

use_groundwater_flow -> groundwater
use_groundwater_storage -> groundwater
use_interception -> interception
use_interflow -> interflow
use_simple_infiltration -> simple_infiltration
use_vegetation_drag_2d -> vegetation_drag_2d

Simulation template settings

use_0d_inflow -> surface and dry_weather_flow
use_structure_control -> table_control and memory_control

  • If the source attributes for the tables are all empty (NULL or ''), no table should be created. E.g. if v2_global_settings.interception_global and .interception_file are both empty, interception should not have any rows
  • If they are not empty, the corresponding use_* attribute should be set to True
@margrietpalm
Copy link
Contributor

Please clarify:

If the source attributes for the tables are all empty (NULL or ''), no table should be created. E.g. if v2_global_settings.interception_global and .interception_file are both empty, interception should not have any rows

Do you want an empty table of no table?

@margrietpalm
Copy link
Contributor

Moved

- [ ] The model checker should give a INFO message if the table has data, but the corresponding `use_*` attribute is False
- [ ]  The model checker should give an ERROR if the `use_*` attribute is True, but the corresponding table has no data

to https://app.zenhub.com/workspaces/team-3di-5ef60eff1973dd0024268b90/issues/gh/nens/threedi-modelchecker/412

@leendertvanwolfswinkel
Copy link
Author

Do you want an empty table of no table?

Empty table

@margrietpalm margrietpalm self-assigned this Nov 25, 2024
@margrietpalm
Copy link
Contributor

margrietpalm commented Nov 25, 2024

It looks like the migration needs to be refined. Currently, this happens:

property migration rule for use_ migration rule for table
use_interflow True if interflow_settings_id is not NULL or "" remove rows not matching interflow_settings_id
use_structure_control True if control_group_id is not NULL or "" remove rows not matching control_group_id
use_simple_infiltration True if simple_infiltration_settings_id is not NULL or "" remove rows not matching simple_infiltration_settings_id
use_vegetation_drag_2d True if vegetation_drag_settings_id is not NULL or "" remove rows not matching vegetation_drag_settings_id
use_groundwater_storage True if groundwater_settings_id is not NULL or "" remove rows not matching groundwater_settings_id
use_groundwater_flow True if groundwater.groundwater_hydraulic_conductivity and groundwater.groundwater_hydraulic_conductivity_file are not NULL nothing
use_interception True if interception.interception != NULL and interception.interception_file is not NULL or "" nothing
use_0d_inflow copied from global surface and dry weather flow are not populated when use_0d_inflow is False

This should be extended with:

  • a check if the found row contains actual data, if not change settings
  • dropping interception contents when use_interception = False in migration 223

@margrietpalm
Copy link
Contributor

@leendertvanwolfswinkel can you confirm that the setting of use_groundwater_flow is done correctly. Currently this sql is executed:

        UPDATE model_settings
        SET use_groundwater_flow = CASE
            WHEN (SELECT groundwater_hydraulic_conductivity FROM groundwater LIMIT 1) IS NOT NULL OR (SELECT groundwater_hydraulic_conductivity_file FROM groundwater LIMIT 1) IS NOT NULL THEN 1
            ELSE 0
        END;

And this happens after setting use_groundwater_storage and removing any rows in groundwater that do not match the id. This implies that if use_groundwater_storage = False, use_groundwater_flow = False as well.

@margrietpalm
Copy link
Contributor

@leendertvanwolfswinkel are you really sure about this?

If they are not empty, the corresponding use_* attribute should be set to True

Imagine a user sets use_* to False but doesn't remove the relating data, this would change that setting back to True. And I don't think this is the intended behavior.

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

No branches or pull requests

2 participants