Conversation
This fixes previously not working parallel computations and introduces a simpler handling of the pool. The users now only need to specify the number of processes.
# Conflicts: # climada/engine/unsequa/calc_cost_benefit.py # climada/engine/unsequa/calc_impact.py # climada/engine/unsequa/test/test_unsequa.py
* Allow to set loglevel * Add method to sort samples * Update CHANGELOG.md * Add advanced examples for unsequa * Remove logging control * Update changelog * Remove tipo * Clarify docstring * Correct docstring * Update t.o.c. * Remove unecessary output prints * Remove linter issues * Update changelog --------- Co-authored-by: Chahan Kropf <[email protected]>
peanutfun
left a comment
peanutfun
left a comment
There was a problem hiding this comment.
It looks good to me but I cannot really assess if this is a clear improvement. My main trouble is the amount of doubled code and the lack of documentation regarding the parallelization process. Other than that, I see no major issues 👍
| samples_df.iterrows(), | ||
| chunksize=chunksize) | ||
|
|
||
| p_iterator = self._sample_parallel_iterator( |
There was a problem hiding this comment.
Suggestion: Make the entire iterator-computation-datamerge part a (local?) function that only takes a dataframe and the process number as arguments. Then you can pass the first DF row with processes=1 to estimate the compute time and afterwards pass the rest of the dataframe with processes=processes. No need to double the code.
There was a problem hiding this comment.
With "local" I meant a function that is defined (only) in the scope of uncertainty. This way, it would need fewer parameters. But it works this way nonetheless
Co-authored-by: Lukas Riedel <[email protected]>
Co-authored-by: Lukas Riedel <[email protected]>
Co-authored-by: Lukas Riedel <[email protected]>
Co-authored-by: Lukas Riedel <[email protected]>
Co-authored-by: Lukas Riedel <[email protected]>
|
Edit: Follow-up issue: #774 |
3 participants
Changes proposed in this PR:
This PR is a small maintenance of the unsequa module.
Note: merge conflict is dependent on changes in PR #762 . Will be solved after the later is merged.SOLVED.impact.tot_value(optional, can be reversed if this should be done when the deprecated method is removed)This PR fixes
unsequamodule unconditionally adjusts matplotlib style #758PR Author Checklist
develop)PR Reviewer Checklist