-
Notifications
You must be signed in to change notification settings - Fork 41
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
Improve Configuration and Prediction Usability #220
base: main
Are you sure you want to change the base?
Improve Configuration and Prediction Usability #220
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty cool changes. I like the much more flexible config design and the huge improvements to the forecast management from different data sources.
- I would personally prefer a nested config as it's imho more readable and avoid name conflicts. Prefixes would allow flat access (and display in the helper app).
- I don't quite understand how to start the test server automatically, i had to manually run fasthtml_server (and how would I access the endpoints over the fastapi, i just accessed it separately?).
- I don't quite see the usecase for the fasthtml yet. Isn't it enough for the config and then restart on changes which shouldn't happen often? Or just use the /config API for changes/display.
- Tests should not modify files outside test/tmp.
- Could you use default= for the Field default values (mypy doesn't like it without)
- Unfortunately a little bit tedious to review such a big PR (could be logically separated into smaller PRs).
|
||
class SettingsEOS( | ||
ConfigCommonSettings, | ||
DevicesCommonSettings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we get problems with overlapping fields? Could we not make a nested structure (could still do with env by using prefixes for each nested level)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not get problems with overlapping fields as the different settings are disjunctive by design and intention. This for sure has to be mentioned in the documentation.
return bool(re.match(iso8601_pattern, value)) | ||
|
||
|
||
class PydanticBaseModel(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to add the numpy serialization here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this would be the place to add any special type serialization.
@@ -200,6 +246,9 @@ def fastapi_optimize( | |||
if start_hour is None: | |||
start_hour = datetime.now().hour | |||
|
|||
config_eos.update() | |||
prediction_eos.update(start_datetime=start_hour) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we change global state here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this changes the global state. It has to be removed when the prediction is connected to the EMS.
|
||
|
||
@pytest.fixture | ||
def config_default_dirs(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably want the tests to operate in temporary directory and don´t interfere with system files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a better solution would be to fake root path for config dirs to be on a temp dir. Just currently don't know how to do that.
assert config_eos._config_folder_path() is None | ||
|
||
config_file_user = config_default_dir_user.joinpath(config_eos.CONFIG_FILE_NAME) | ||
shutil.copy2(config_eos.config_default_file_path, config_file_user) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please no interference between test and system.
The optimize tests fail due to empty available_charging_rates_in_percentage array -> we probably want pydantic to check that input. |
The pvforecastakkudoktor fails in combination with test_prediction.py (I didn't investigate further, maybe something stateful with the fixtures). |
for fmt in formats: | ||
try: | ||
# We assume values without timezone are specified in local date and time. | ||
dt = pendulum.from_format(date_input, fmt, tz=pendulum.local_timezone()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the Github pipeline fails because it has UTC as local timezone, should probably take in_timezone here if set.
Btw. pendulum doesn't provide wheels for Python 3.13 yet (see sdispater/pendulum#844) so our Docker build would break with 3.13 as we use python-slim images which are based on Debian bookworm atm (so even if we would install rust from the sources there, the build would fail because rust is too old. So we would need to install a current rustc to make it work with 3.13). |
Yes, this is a common problem at the moment. In another project I just switched to 3.12.9. Not as fast but packet availability is much better. |
Got the culprit. #141 removed the default values. Will re-add the default values. |
Make reading config parameters from environment vars more complex. For the moment I want to stay with a flat config design.
Sorry, when changing to the lifespan context I forgot to add it to the app. Will be changed in the next round.
Future use case for fasthtml is the configuration of PV systems adding parameters from a module/ inverter database. Not needed for now. Another use case would be to serve the documentation (not "only" the openai interface).
Yes. Still not so easy as there are default search paths that cannot easily be redirected. Have to think about that.
Sure.
Maybe you could review commit by commit. I tried to make commits contain limited context. |
f6e9b65
to
4675ec1
Compare
5c645cd
to
307941d
Compare
is there anything we can help you with at this point? |
Thanks for your kind offer. These are the remaining mypy errors.
Patches/ Pull Requests are welcome. src/akkudoktoreos/core/ems.py:166: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:167: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:168: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:170: error: Incompatible types in assignment (expression has type "list[float] | ndarray[Any, dtype[float]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:172: error: Value of type variable "_SCT" of "full" cannot be "float" [type-var]
src/akkudoktoreos/core/ems.py:172: error: Argument 1 to "len" has incompatible type "list[float] | None"; expected "Sized" [arg-type]
src/akkudoktoreos/core/ems.py:174: error: Item "None" of "Wechselrichter | None" has no attribute "akku" [union-attr]
src/akkudoktoreos/core/ems.py:178: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:179: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:180: error: Incompatible types in assignment (expression has type "ndarray[Any, dtype[Any]]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:183: error: Item "None" of "PVAkku | None" has no attribute "set_discharge_per_hour" [union-attr]
src/akkudoktoreos/core/ems.py:183: error: Argument 1 to "set_discharge_per_hour" of "PVAkku" has incompatible type "list[int]"; expected "ndarray[Any, Any]" [arg-type]
src/akkudoktoreos/core/ems.py:186: error: Incompatible types in assignment (expression has type "ndarray[Any, Any]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:189: error: Incompatible types in assignment (expression has type "ndarray[Any, Any]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:192: error: Incompatible types in assignment (expression has type "list[int]", variable has type "list[float] | None") [assignment]
src/akkudoktoreos/core/ems.py:195: error: Item "None" of "HomeAppliance | None" has no attribute "set_starting_time" [union-attr]
src/akkudoktoreos/core/ems.py:195: error: Argument 1 to "set_starting_time" of "HomeAppliance" has incompatible type "list[int]"; expected "int" [arg-type]
src/akkudoktoreos/core/ems.py:197: error: Return type "None" of "reset" incompatible with return type "PydanticBaseModel" in supertype "PydanticBaseModel" [override]
src/akkudoktoreos/core/ems.py:200: error: Item "None" of "PVAkku | None" has no attribute "reset" [union-attr]
src/akkudoktoreos/core/ems.py:223: error: Item "None" of "Any | None" has no attribute "hour" [union-attr]
src/akkudoktoreos/core/ems.py:225: error: Function is missing a return type annotation [no-untyped-def]
src/akkudoktoreos/core/ems.py:252: error: Argument 1 to "len" has incompatible type "list[float] | None"; expected "Sized" [arg-type]
src/akkudoktoreos/core/ems.py:253: error: Argument 1 to "len" has incompatible type "list[float] | None"; expected "Sized" [arg-type]
src/akkudoktoreos/core/ems.py:256: error: Argument 1 to "len" has incompatible type "list[float] | None"; expected "Sized" [arg-type]
src/akkudoktoreos/core/ems.py:271: error: Item "None" of "PVAkku | None" has no attribute "ladezustand_in_prozent" [union-attr]
src/akkudoktoreos/core/ems.py:279: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:287: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:289: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:297: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:298: error: Item "None" of "PVAkku | None" has no attribute "set_charge_allowed_for_hour" [union-attr]
src/akkudoktoreos/core/ems.py:298: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:300: error: Item "None" of "Wechselrichter | None" has no attribute "energie_verarbeiten" [union-attr]
src/akkudoktoreos/core/ems.py:304: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:305: error: Item "None" of "PVAkku | None" has no attribute "set_charge_allowed_for_hour" [union-attr]
src/akkudoktoreos/core/ems.py:306: error: Item "None" of "PVAkku | None" has no attribute "energie_laden" [union-attr]
src/akkudoktoreos/core/ems.py:307: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:321: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:324: error: Value of type "list[float] | None" is not indexable [index]
src/akkudoktoreos/core/ems.py:328: error: Item "None" of "PVAkku | None" has no attribute "ladezustand_in_prozent" [union-attr]
src/akkudoktoreos/core/ems.py:357: error: Function is missing a return type annotation [no-untyped-def]
tests/test_class_ems_2.py:28: error: Argument "hours" to "PVAkku" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems_2.py:39: error: Argument "hours" to "HomeAppliance" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems_2.py:46: error: Argument "hours" to "PVAkku" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems_2.py:50: error: Unsupported operand types for * ("list[float]" and "int | None") [operator]
tests/test_class_ems_2.py:54: error: Unsupported operand types for * ("list[float]" and "int | None") [operator]
tests/test_class_ems_2.py:128: error: Argument 1 to "full" has incompatible type "int | None"; expected "SupportsIndex | Sequence[SupportsIndex]" [arg-type]
tests/test_class_ems_2.py:131: error: Argument 1 to "full" has incompatible type "int | None"; expected "SupportsIndex | Sequence[SupportsIndex]" [arg-type]
tests/test_class_ems.py:29: error: Argument "hours" to "PVAkku" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems.py:40: error: Argument "hours" to "HomeAppliance" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems.py:47: error: Argument "hours" to "PVAkku" has incompatible type "int | None"; expected "int" [arg-type]
tests/test_class_ems.py:49: error: Argument 1 to "full" has incompatible type "int | None"; expected "SupportsIndex | Sequence[SupportsIndex]" [arg-type]
src/akkudoktoreos/optimization/genetic.py:342: error: Argument 1 to "split_individual" of "optimization_problem" has incompatible type "list[float]"; expected "list[int]" [arg-type]
src/akkudoktoreos/optimization/genetic.py:382: error: Argument 1 to "split_individual" of "optimization_problem" has incompatible type "list[float]"; expected "list[int]" [arg-type]
src/akkudoktoreos/optimization/genetic.py:518: error: Argument 2 to "setup_deap_environment" of "optimization_problem" has incompatible type "int | None"; expected "int" [arg-type]
src/akkudoktoreos/optimization/genetic.py:521: error: Argument 3 to "evaluate" of "optimization_problem" has incompatible type "int | None"; expected "int" [arg-type]
src/akkudoktoreos/optimization/genetic.py:550: error: Argument 9 to "visualisiere_ergebnisse" has incompatible type "int | None"; expected "int" [arg-type] |
do you think we should fix this in a new PR or inside this one? |
The remaining errors should be fixed inside here. The basic problem is the data that is hold by the EnergyManagementSystem. Internally it is numpy arrays. In the parameters it is just lists. I took the list approach to add the data to the EnergaManagementSystem class (which is a pydantic base model), that is why mypy is complaining. If this data would be taken from the new prediction, the storage in the EnergyManagementSystem class is not needed anymore. So it is here to be replaced.
The predictions are still not connected to the EnergyManagementSystem. It needs some architecture design to do that. Besides that the configuration framework and the predictions are ready for review.
I don't think so. |
307941d
to
16a3967
Compare
I will now have a look into the remaining mypy errors. Hopefully after that the PR is ready for merge. The EnergyManagementSystem will still not get the data from the new predictions. This change should be done in another PR. |
16a3967
to
4ff0078
Compare
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
4ff0078
to
2d690e1
Compare
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
The PV forecast modules are adapted from the class_pvforecast module and replace it. Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
The modules provide classes and methods to retrieve, manage, and process weather forecast data from various sources. Includes are structured representations of weather data and utilities for fetching forecasts for specific locations and time ranges. BrightSky and ClearOutside are currently supported. Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
2d690e1
to
d13d31c
Compare
Ready for Review.
All test are passing. The optimization and visualisation tests were not changed, so hopefully there are no new bugs introduced in the current logic. A lot of MyPy complaints were worked on. There is one special change that MyPy was requesting. The type of individual hat to be changed from list[int] to list[float] in genetic.py, but only on some places. I don't know whether this is right. Please have a look on that. Cheers |
Improve Configuration and Prediction Usability
The story behind this pull request is: I wanted to add a weather prediction to EOS ...
During the development of this prediction, I found that the EOS configuration and prediction modules
offered limited support for my use case. This pull request aims to address and enhance that support.
Configuration Requirements and User Stories
Configuration properties play a crucial role across the EOS system, as they are essential for all sub-modules. However, there are several distinct stakeholders with varying needs when it comes to configuration management:
Submodules:
Submodules like devices, prediction, and optimization rely on specific configuration properties. While these modules are primarily consumers of the configuration, it is rare for them to initiate changes to configuration properties directly.
Users:
Users need the ability to configure the entire EOS system through various methods, such as configuration files, environment variables, and the server's REST API. They also require access to the current configuration and should be able to update it not only at server startup but also dynamically during runtime. Additionally, users may benefit from a GUI tool to simplify the creation and management of configuration files.
Developers:
Developers want a straightforward process for adding new configuration properties to support new features. The complexity of managing configuration logic should be abstracted away, allowing developers to programmatically modify configuration properties without needing to understand how they are consumed by the system.
Testers:
Testers need a convenient way to programmatically set up configuration properties for test runs. They also require the ability to reset configuration properties to a defined state to ensure consistent and repeatable testing environments.
Design Implications
Based on these requirements, configuration properties should be simple values. This ensures they can be easily represented in formats like environment variables, making them flexible and accessible for all stakeholders.
Prediction Requirements and User Stories
Predictions are an essential component of the EOS system, serving the needs of optimization, device simulations, and other prediction modules. Various stakeholders have specific requirements for how predictions should be managed and accessed:
Submodules:
Submodules such as optimization, devices, and prediction modules need seamless access to all prediction properties. These properties are typically required as time-series data. In some cases, access to historical prediction data may be necessary for comparison with actual measurements.
Users:
Users require read-only access to prediction properties, often via the REST API. Additionally, users may want to provide custom prediction data—either through file uploads or the REST API—for use in subsequent optimization runs.
Developers:
Developers need a streamlined way to add new prediction functionalities without having to build core prediction logic from scratch. A robust prediction framework should enable developers to quickly and safely extend the system with new prediction providers.
Testers:
Testers need the ability to preset predictions using synthetic data. This could involve loading data from files or generating it programmatically to create controlled test environments.
Design Implications
To address these requirements, the prediction framework should ensure:
Configuration Design
The EOS configuration is implemented as a singleton, ensuring all stakeholders access the same shared configuration properties. The configuration is composed of settings aggregated across the system. These settings can be defined anywhere within EOS, with the configuration acting as a centralized container for them. Configuration property values are derived from four primary sources, prioritized as follows:
If a property is defined in multiple sources, the priority order above determines which value takes precedence.
Accessing Configuration
Access to the configuration is provided by the
get_config()
function, which works similarly toget_logger()
. For added convenience, a base class allows accessing the configuration via theconfig
attribute. Configuration properties are exposed as attributes of the singleton. Both the settings and the aggregated configuration are implemented using Pydantic models.Adding New Settings
For developers, there is a single base class:
SettingsBase
. All new settings must derive from this class. Once created, the newSettings
class is added to the configuration to make it accessible across EOS. That’s all that is required to extend the configuration.Internal Structure
The configuration submodule consists of:
configabc.py
): Provide the foundational design for settings and configuration.config.py
): Implements the aggregation and access logic.Prediction Design
The EOS prediction system is also a singleton, ensuring unified access to prediction data across all stakeholders. The prediction system acts as a container for prediction providers, centralizing their access while abstracting the implementation details. Predictions behave like a key-value store, where:
Handling Multiple Providers
Different providers can supply the same prediction properties. In such cases, a specific provider can be selected through configuration using its unique provider ID (e.g.,
"weather_provider": "ClearOutside"
).Accessing Predictions
The
get_prediction()
function provides access to the prediction system. For convenience, a base class allows accessing predictions through theprediction
attribute. Prediction records, providers, and the prediction container itself are implemented as Pydantic models.Prediction Components
Predictions are organized into lists of prediction records, where each record belongs to a provider. A prediction provider is responsible for managing its respective records. This modular design simplifies the addition of new prediction types and providers.
Types of Predictions
Abstract and base classes are provided for general prediction handling and specific prediction types:
Real Prediction Providers
Concrete providers derived from the base classes include:
ElecPriceAkkudoktor
: Fetches data from the Akkudoktor.net API.ElecPriceImport
: Imports data from files.LoadAkkudoktor
: Import load data from Akkudoktor load profiles.LoadImport
: Imports load data from files.PVForecastAkkudoktor
: Fetches PV forecasts from Akkudoktor.net.PVForecastImport
: Imports PV forecast data from files.BrightSky
: Retrieves weather data via an online JSON API.ClearOutside
: Scrapes weather data from a web source.WeatherImport
: Imports weather data from files.Summary
This architecture ensures that:
Experimental Application Server
To support GUI-based configuration, an independent application server has been introduced. This server operates separately from the FastAPI REST server, ensuring modularity and separation of concerns.
The FastAPI server proxies any unhandled requests to the application server. This approach ensures that users perceive a single unified server while preserving the application's design flexibility and independence.
The application server is a FastHTML server that enables the development of web applications completely in Python. Currently, its functionality is limited to displaying configuration data.
The application server is started automatically during the FastAPI server's startup process. Alternatively, it can also be launched independently.
Testing
Most modules are now covered by tests. New test fixtures in
conftest.py
have been introduced to streamline configuration management, including stashing and resetting configuration files.Since the FastAPI server now serves configuration and prediction endpoints, it provides a convenient way to explore and test the application’s functionality.
Changes to the Existing Design
Configuration
The configuration system has undergone significant redesign. While the JSON configuration file is still used, its format has been updated to a flat dictionary structure. The existing configuration logic was moved into a new
config
submodule. Tests were adapted to align with the original test cases wherever possible.Prediction
Prediction functionality was mostly ported to the new design without altering its core logic. Notable changes include:
PVForecastAkkudoktor
: Enhanced the 'PV_Forecast' to allow configuration of PV system properties directly in the settings, rather than through the URL.XxxxImport
): These now centralizingload_from_file
functions, that wrere previously scattered.EMS (Energy Management System)
The
ems
module did not really fit under the prediction category. It is the core module for EOS, combining devices, predictions, and optimization. It now resides in its owncore
submodule.Devices and Optimization
These submodules were minimally impacted, mostly configuration is now taken from the centralized EOS configuration. No other/ functional changes. Abstract and base classes were introduced but remain placeholders for future expansion.
Server Enhancements
The server retains all its original functionality, with enhancements for proxying and additional features to provide access to the configuration and prediction.
Date and Time Utilities
The datetime utilities were migrated to Pendulum, providing robust handling of daylight saving time changes. All datetime values are now timezone-aware by default. A new
compare_datetimes
function enables comparisons across different timezones.Current State
EnergyManagementSystem
(EMS).EnergyManagementSystemParameters
for inputs. Access to config and prediction is already given.Open Items
The
EnergyManagementSystem
class has access to configuration and prediction but does not yet utilize the prediction module.ValueError: empty range in randrange(0, 0)
intest_optimize
.Instability in OpenAPI JSON generation, possibly due to proxying in the FastAPI server.
test_timezone_behaviour
fails inconsistently, passing as a standalone test but failing when other tests run first.Implement feed-in tariff prediction to supplement all of the
EnergyManagementSystemParameters
.FLW
Feedback and suggestions are welcome. Please review the current design and implementation. Specific hints regarding unresolved test failures or design improvements would be particularly appreciated.