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

Support schema 300 (settings) #366

Merged
merged 63 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
aacb342
Fix all so modelchecker can be imported
margrietpalm Mar 15, 2024
ab1239d
Fix typos in factories
margrietpalm Mar 15, 2024
505a34f
Replace empty gpkg with geopackage produced with schema 0.220
margrietpalm Mar 15, 2024
00dfd3f
Remove non-existing attributes from ModelSettingsFactory
margrietpalm Mar 15, 2024
7643a03
Remove var_name from AggregationSettingsFactory
margrietpalm Mar 15, 2024
5bd3730
fix test_unique_check_multiple_description
margrietpalm Mar 15, 2024
f314808
Skip failing tests and add todos for fixing
margrietpalm Mar 15, 2024
2cf4a9b
Fix test_factories
margrietpalm Mar 15, 2024
900eb5c
Fix typos
margrietpalm Mar 15, 2024
95e8272
Fix CorrectAggregationSettingsExist
margrietpalm Mar 15, 2024
45b2f36
Fix test_beta_columns
margrietpalm Mar 15, 2024
9186f40
Disable a bunch of broken checks
margrietpalm Mar 18, 2024
36d51eb
Add (commented-out) geometry casting to test_defined_area
margrietpalm Mar 18, 2024
ef307e3
Might fix multipolygon issue(?)
jpprins1 Mar 18, 2024
edcab4f
try threedi-schema from github
jpprins1 Mar 18, 2024
8c5ae3d
fix linting
jpprins1 Mar 18, 2024
029a869
correct dependency
jpprins1 Mar 18, 2024
bd5442c
remove most usages of groundwater_filter
margrietpalm Mar 18, 2024
aa769c5
remove most usages of interflow_filter
margrietpalm Mar 18, 2024
8722094
remove most usages of infiltration_filter
margrietpalm Mar 18, 2024
b8844ce
remove most usages of vegetation_drag_filter
margrietpalm Mar 18, 2024
c004c2a
remove most usages of first_setting_filter
margrietpalm Mar 18, 2024
70b8d82
Remove filters from RASTER_COLUMNS_FILTERS and remove unused settings…
margrietpalm Mar 18, 2024
4d72547
Fix some more checks and run black
margrietpalm Mar 18, 2024
3cbfb21
Fix more checks
margrietpalm Mar 18, 2024
1cdfc87
Fix formatting
margrietpalm Mar 18, 2024
11426a9
Merge branch 'master' into margriet_363_schema_300_settings_v2
margrietpalm Mar 18, 2024
46c59e3
Fix latest changes from master
margrietpalm Mar 18, 2024
ed2dd98
Fix simple infiltration and groundwater combi check
margrietpalm Mar 19, 2024
ab50997
Fix tests to ensure that settings are present when use_foo is true fo…
margrietpalm Mar 19, 2024
180a157
Fix a bunch of todos, mainly tests
margrietpalm Mar 20, 2024
78a8e11
Add test for Use0DFlowCheck
margrietpalm Mar 20, 2024
f8cfc1b
Add test for UsedSettingsPresentCheck
margrietpalm Mar 20, 2024
526a60b
Add test for SettingsLengthCheck
margrietpalm Mar 20, 2024
a8fb769
Add test for AggregationSettingsInvervalCheck
margrietpalm Mar 20, 2024
a400e34
Remove comment
margrietpalm Mar 20, 2024
3cc5d4b
Remove any reference to first_setting_filter and similar
margrietpalm Mar 20, 2024
a2b7106
Remove some columns from RASTER_COLUMNS that should not have been there
margrietpalm Mar 20, 2024
326ed30
Rename some table and column names in strings
margrietpalm Mar 25, 2024
6f2cf8c
Fix check
margrietpalm Mar 25, 2024
c2ce0fe
Fix bug in description
margrietpalm Mar 26, 2024
613b93d
Fix AggregationSettingsInvervalCheck
margrietpalm Mar 26, 2024
83275c8
Merge branch 'margriet_363_schema_300_settings_v2' of github.com:nens…
margrietpalm Mar 27, 2024
54ffd86
Fix settings table migration, I think
margrietpalm Mar 27, 2024
5ffa118
Simplify SettingsLengthCheck to MaxOneRecordCheck
margrietpalm Apr 8, 2024
2c37bd1
Remove bunch of commented filters
margrietpalm Apr 8, 2024
a77f899
Remove AllEqualCheck for model settings table
margrietpalm Apr 8, 2024
b8d8bc4
Replace AggregationSettingsInvervalCheck with a generic check
margrietpalm Apr 8, 2024
4a674d3
Replace mention of initial_settings with initial_conditions and doubl…
margrietpalm Apr 8, 2024
7d32202
Change number of MaxOneRecordCheck from 326 to 328
margrietpalm Apr 8, 2024
59721bd
Update threedi_modelchecker/checks/other.py
margrietpalm Apr 8, 2024
4acf4dd
Fix (local) paths to raster files
margrietpalm Apr 9, 2024
474ca62
Correct wrong (and confusing) typehint
margrietpalm Apr 9, 2024
c9e62b4
Merge branch 'margriet_363_schema_300_settings_v2' of github.com:nens…
margrietpalm Apr 9, 2024
655c8ae
Use pypi package
margrietpalm Apr 9, 2024
bcd83a6
Upgrade threedi version and upgrade versin
margrietpalm Apr 29, 2024
274041b
up threedi-schema and dev version
margrietpalm Apr 29, 2024
07d3cc9
Add mapping between the fields for raster files in the schema and Ras…
margrietpalm Apr 30, 2024
418fdcc
Merge branch 'master' into margriet_363_schema_300_settings_v2
margrietpalm May 22, 2024
2b01f19
Merge branch 'master' into margriet_363_schema_300_settings_v2
margrietpalm May 22, 2024
3795e8d
Update changes
margrietpalm May 22, 2024
8516acf
Use latest threedi-schema
margrietpalm May 22, 2024
f14e2c8
Update pyproject.toml
margrietpalm May 22, 2024
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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ dependencies = [
"Click",
"GeoAlchemy2>=0.9,!=0.11.*",
"SQLAlchemy>=1.4",
"threedi-schema==0.220.*"
"threedi-schema@https://github.com/nens/threedi-schema/archive/refs/heads/margriet_46_schema_300.zip"
margrietpalm marked this conversation as resolved.
Show resolved Hide resolved
]

[project.optional-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion threedi_modelchecker/checks/geo_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


def epsg_code_query():
epsg_code = Query(models.GlobalSetting.epsg_code).limit(1).scalar_subquery()
epsg_code = Query(models.ModelSettings.epsg_code).limit(1).scalar_subquery()
return func.coalesce(epsg_code, literal(DEFAULT_EPSG)).label("epsg_code")


Expand Down
125 changes: 92 additions & 33 deletions threedi_modelchecker/checks/other.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@
from .cross_section_definitions import cross_section_configuration
from .geo_query import distance, length, transform

# Use these to make checks only work on the first global settings entry:
first_setting = (
Query(models.GlobalSetting.id)
.order_by(models.GlobalSetting.id)
.limit(1)
.scalar_subquery()
)
first_setting_filter = models.GlobalSetting.id == first_setting

margrietpalm marked this conversation as resolved.
Show resolved Hide resolved

class CorrectAggregationSettingsExist(BaseCheck):
"""Check if aggregation settings are correctly filled with aggregation_method and flow_variable as required"""
Expand All @@ -30,25 +21,19 @@ def __init__(
*args,
**kwargs,
):
super().__init__(column=models.GlobalSetting.id, *args, **kwargs)
super().__init__(column=models.ModelSettings.id, *args, **kwargs)
self.aggregation_method = aggregation_method.value
self.flow_variable = flow_variable.value

def get_invalid(self, session: Session) -> List[NamedTuple]:
global_settings = self.to_check(session).filter(first_setting_filter)
global_settings = self.to_check(session)
correctly_defined = session.execute(
select(models.AggregationSettings)
.filter(
select(models.AggregationSettings).filter(
models.AggregationSettings.aggregation_method
== self.aggregation_method,
models.AggregationSettings.flow_variable == self.flow_variable,
)
.filter(
models.AggregationSettings.global_settings_id
== global_settings.subquery().c.id
)
).all()

return global_settings.all() if len(correctly_defined) == 0 else []

def description(self) -> str:
Expand Down Expand Up @@ -244,12 +229,14 @@ class Use0DFlowCheck(BaseCheck):
"""

def __init__(self, *args, **kwargs):
super().__init__(column=models.GlobalSetting.use_0d_inflow, *args, **kwargs)
super().__init__(
column=models.SimulationTemplateSettings.use_0d_inflow, *args, **kwargs
)

def to_check(self, session):
"""Return a Query object on which this check is applied"""
return session.query(models.GlobalSetting).filter(
models.GlobalSetting.use_0d_inflow != 0
return session.query(models.SimulationTemplateSettings).filter(
models.SimulationTemplateSettings.use_0d_inflow != 0
)

def get_invalid(self, session):
Expand Down Expand Up @@ -301,7 +288,7 @@ def get_invalid(self, session):
class ConnectionNodesLength(BaseCheck):
"""Check that the distance between `start_node` and `end_node` is at least
`min_distance`. The coords will be transformed into (the first entry) of
GlobalSettings.epsg_code.
ModelSettingss.epsg_code.
margrietpalm marked this conversation as resolved.
Show resolved Hide resolved
"""

def __init__(
Expand Down Expand Up @@ -469,7 +456,7 @@ def description(self) -> str:

class OpenChannelsWithNestedNewton(BaseCheck):
"""Checks whether the model has any closed cross-section in use when the
NumericalSettings.use_of_nested_newton is turned off.
NumericalSettings.use_nested_newton is turned off.

See https://github.com/nens/threeditoolbox/issues/522
"""
Expand All @@ -479,7 +466,7 @@ def __init__(self, level=CheckLevel.WARNING, *args, **kwargs):
column=models.CrossSectionDefinition.id,
level=level,
filters=Query(models.NumericalSettings)
.filter(models.NumericalSettings.use_of_nested_newton == 0)
.filter(models.NumericalSettings.use_nested_newton == 0)
.exists(),
*args,
**kwargs,
Expand Down Expand Up @@ -532,8 +519,8 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
def description(self) -> str:
return (
f"{self.column_name} has a closed cross section definition while "
f"NumericalSettings.use_of_nested_newton is switched off. "
f"This gives convergence issues. We recommend setting use_of_nested_newton = 1."
f"NumericalSettings.use_nested_newton is switched off. "
f"This gives convergence issues. We recommend setting use_nested_newton = 1."
)


Expand Down Expand Up @@ -731,7 +718,8 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
.filter(
(models.ConnectionNode.storage_area != None)
& (
( # calculate how many seconds the pumpstation takes to empty its storage: (storage * height)/pump capacity
(
# calculate how many seconds the pumpstation takes to empty its storage: (storage * height)/pump capacity
(
# Arithmetic operations on None return None, so without this
# conditional type cast, no invalid results would be returned
Expand All @@ -744,9 +732,7 @@ def get_invalid(self, session: Session) -> List[NamedTuple]:
)
)
/ (models.Pumpstation.capacity / 1000)
< Query(models.GlobalSetting.sim_time_step)
.filter(first_setting_filter)
.scalar_subquery()
< Query(models.TimeStepSettings.time_step).scalar_subquery()
)
)
.all()
Expand Down Expand Up @@ -819,7 +805,7 @@ class InflowNoFeaturesCheck(BaseCheck):
"""Check that the surface table in the global use_0d_inflow setting contains at least 1 feature."""

def __init__(self, *args, surface_table, condition=True, **kwargs):
super().__init__(*args, column=models.GlobalSetting.id, **kwargs)
super().__init__(*args, column=models.ModelSettings.id, **kwargs)
self.surface_table = surface_table
self.condition = condition

Expand All @@ -828,13 +814,13 @@ def get_invalid(self, session: Session):
select(func.count(self.surface_table.id))
).scalar()
return (
session.query(models.GlobalSetting)
session.query(models.ModelSettings)
.filter(self.condition, surface_table_length == 0)
.all()
)

def description(self) -> str:
return f"v2_global_settings.use_0d_inflow is set to use {self.surface_table.__tablename__}, but {self.surface_table.__tablename__} does not contain any features."
return f"model_settings.use_0d_inflow is set to use {self.surface_table.__tablename__}, but {self.surface_table.__tablename__} does not contain any features."


class NodeSurfaceConnectionsCheck(BaseCheck):
Expand Down Expand Up @@ -1082,3 +1068,76 @@ def _get_records(self, session):
)
)
)


class UsedSettingsPresentCheck(BaseCheck):
def __init__(
self,
column,
settings_table,
filters=None,
level=CheckLevel.ERROR,
error_code=0,
):
super().__init__(column, filters, level, error_code)
self.settings_table = settings_table

def get_invalid(self, session: Session) -> List[NamedTuple]:
# more than 1 row should be caught by another check
all_results = self.to_check(session).filter(self.column == True).all()
use_cols = len(all_results) > 0
if use_cols and session.query(self.settings_table).count() == 0:
return all_results
return []

def description(self) -> str:
return f"{self.column_name} in {self.table.name} is set to True but {self.settings_table.name} is empty"


class SettingsLengthCheck(BaseCheck):
def __init__(
self,
column,
filters=None,
level=CheckLevel.ERROR,
error_code=0,
min_length=0,
margrietpalm marked this conversation as resolved.
Show resolved Hide resolved
max_length=1,
):
super().__init__(column, filters, level, error_code)
self.min_length = min_length
self.max_length = max_length
self.observed_length = 0

def get_invalid(self, session: Session) -> List[NamedTuple]:
# return mock list in case the table is empty when it shouldn't be
all_results = self.to_check(session).all()
self.observed_length = len(all_results)
if (self.observed_length > self.max_length) or (
self.observed_length < self.min_length
):
return all_results if self.observed_length > 0 else ["foo"]
else:
return []

def description(self) -> str:
return (
f"{self.table.name} has {self.observed_length} rows, "
f"but should have at least {self.min_length} rows and "
f"at most {self.max_length} rows."
)


class AggregationSettingsInvervalCheck(BaseCheck):
def get_invalid(self, session: Session) -> List[NamedTuple]:
return (
session.query(models.AggregationSettings, models.TimeStepSettings)
.filter(
models.AggregationSettings.interval
margrietpalm marked this conversation as resolved.
Show resolved Hide resolved
< models.TimeStepSettings.output_time_step
)
.all()
)

def description(self) -> str:
return "aggregation_settings.interval should be smaller than time_step_settings.output_time_step"
18 changes: 10 additions & 8 deletions threedi_modelchecker/checks/raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class RasterHasMatchingEPSGCheck(BaseRasterCheck):
"""Check whether a raster's EPSG code matches the EPSG code in the global settings for the SQLite."""

def get_invalid(self, session):
epsg_code_query = session.query(models.GlobalSetting.epsg_code).first()
epsg_code_query = session.query(models.ModelSettings.epsg_code).first()
if epsg_code_query is not None:
self.epsg_code = epsg_code_query[0]
else:
Expand All @@ -175,7 +175,7 @@ def is_valid(self, path: str, interface_cls: Type[RasterInterface]):
return raster.epsg_code == self.epsg_code

def description(self):
return f"The file in {self.column_name} has no EPSG code or the EPSG code does not match does not match v2_global_settings.epsg_code"
return f"The file in {self.column_name} has no EPSG code or the EPSG code does not match does not match model_settings.epsg_code"


class RasterSquareCellsCheck(BaseRasterCheck):
Expand All @@ -202,9 +202,11 @@ class RasterGridSizeCheck(BaseRasterCheck):
"""Check whether the global settings' grid size is an even multiple of a raster's cell size (at least 2x)."""

def get_invalid(self, session):
grid_space_query = session.query(models.GlobalSetting.grid_space).first()
if grid_space_query is not None:
self.grid_space = grid_space_query[0]
minimum_cell_size_query = session.query(
models.ModelSettings.minimum_cell_size
).first()
if minimum_cell_size_query is not None:
self.minimum_cell_size = minimum_cell_size_query[0]
else:
return []
return super().get_invalid(session)
Expand All @@ -217,17 +219,17 @@ def is_valid(self, path: str, interface_cls: Type[RasterInterface]):
try:
return (
isclose(
a=((self.grid_space / raster.pixel_size[0]) % 2),
a=((self.minimum_cell_size / raster.pixel_size[0]) % 2),
b=0,
rel_tol=1e-09,
)
) and (self.grid_space >= (2 * raster.pixel_size[0]))
) and (self.minimum_cell_size >= (2 * raster.pixel_size[0]))
# if one of the fields is a NoneType it will be caught elsewhere
except TypeError:
return True

def description(self):
return "v2_global_settings.grid_space is not a positive even multiple of the raster cell size."
return "model_settings.minimum_cell_size is not a positive even multiple of the raster cell size."


class RasterPixelCountCheck(BaseRasterCheck):
Expand Down
Loading
Loading