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

refactor the structure of the (internal) dicts: dict_series, dict_predicted,.... #57

Closed
dieuska opened this issue Sep 6, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@dieuska
Copy link
Collaborator

dieuska commented Sep 6, 2024

In the current code, when executing a series of calculation for multiple relevant distances, the result is arranged by relevant distance: (key = relevant distance).
It should be better to always give results arranged on thematic_id (key=thematic_id) because most of the functionalities is based on the fact that we want to do calculations, predictions etc. for all thematic objects.

At the moment this structure has to be re-arranged multiple times (util-function: dict_series_by_keys). If we would do this refactoring, this re-arrangement shouldn't be necessary anymore resulting in less handling.

So I would propose to always give results in following order:
image

This could be a big refactoring as this is used in multiple functions

@dieuska dieuska added enhancement New feature or request question Further information is requested labels Sep 6, 2024
@dieuska dieuska added this to the 0.3.0 milestone Sep 6, 2024
@dieuska
Copy link
Collaborator Author

dieuska commented Sep 6, 2024

@roefem what do you think about this remark?

@roefem
Copy link
Contributor

roefem commented Sep 9, 2024

Seems good to me, I don't see any issues with reordering the way dict_series is composed

@dieuska
Copy link
Collaborator Author

dieuska commented Sep 12, 2024

when executing; to test if this refactoring is really faster? (at least it looks more logic to me to do this refactoring

@dieuska
Copy link
Collaborator Author

dieuska commented Sep 17, 2024

Added 'example_speedtest.py'; an example to measure the speed of execution (10 times) before and after refactoring:

Results:

#BEFORE REFACTORING dict_series

duration: [25.652311, 27.894154, 19.641618, 19.929254, 44.754033, 25.218422, 23.167992, 18.649832, 22.899336, 52.108296]

Min: 18.649832

Max: 52.108296

Mean: 27.9915248

Median: 24.193207

Stdv: 11.28891821173264

#AFTER refactoring

duration: [21.313991, 16.558168, 16.590126, 18.111118, 16.872433, 17.928071, 18.32295, 17.87116, 19.516652, 16.729241]

Min: 16.558168

Max: 21.313991

Mean: 17.981391

Median: 17.8996155

Stdv: 1.504459449440969

Conclusion: refactoring resulted in faster execution-times

dieuska added a commit that referenced this issue Sep 17, 2024
refactoring of the dict_series resulting in multiple changes
dieuska added a commit that referenced this issue Sep 17, 2024
@dieuska dieuska closed this as completed Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants