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

visualize: fix timestamps on diagramms #430

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

celle1234
Copy link
Contributor

I tried to fix the problem with the timestamps in the diagrams. The discussion you can find in #387.

@NormannK
Copy link
Collaborator

visualization_results.pdf
from single_test_optimization.py for discussion

@Lasall
Copy link
Collaborator

Lasall commented Jan 29, 2025

Thank you for your PR! Currently the optimization endpoint (and therefore the visualization) expect the forecast data starting at 00:00 of the current day + x prediction hours (currently 48). The optimization starts at start_hour (default rounded down current hour, e.g. 16:42 -> 16) and goes until optimization hours (currently 48) which starts at 00:00 of the current day just like the predictions (e.g. start_hour = 16, optimization_hours = 48 - > optimization will go until end of the next day).

@celle1234
Copy link
Contributor Author

@Lasall Yes I understand this in the same way.

For the PR it is important, that the timezone in EOS.config.json is set. Otherwise the axis timeline does not match.

@Lasall
Copy link
Collaborator

Lasall commented Jan 30, 2025

new_optimize_result_2_full.pdf
from test artifact.

Maybe you could also adjust the relative times "hours since start" to be the same between predictions and optimization?Currently it could be confusing the prediction goes 0-48 and optimization 0-38 (even though optimization_hours is set to 48).

@celle1234
Copy link
Contributor Author

Did you mean something like this?
visualization_results.pdf

@NormannK
Copy link
Collaborator

NormannK commented Jan 31, 2025

Did you mean something like this? visualization_results.pdf

after some code digging I think this is "more" correct.

cls._start_datetime = start_datetime.set(minute=0, second=0, microsecond=0)

This line does set the start time to the start of the current hour (if not defined). But imo it should be the start of the next hour, but that's something for an other discussion.

@celle1234
Copy link
Contributor Author

I added the changes.

@Lasall
Copy link
Collaborator

Lasall commented Feb 4, 2025

Looks good to me, probably need some fine tuning with the optimization diagram (offset +1) but in a different PR (I think that's also what you meant Normann).

PDFs from test can be checked out here: https://github.com/Akkudoktor-EOS/EOS/actions/runs/13120831990?pr=430

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

Successfully merging this pull request may close these issues.

3 participants