ENH: Display more information in MonteCarlo prints and plots #760
Conversation
CHANGELOG.md
Outdated
| @@ -48,6 +48,7 @@ Attention: The newest changes should be on top --> | |||
|
|
|||
| - REL: update version to 1.7.1 in configuration files [#750](https://github.com/RocketPy-Team/RocketPy/pull/750) | |||
| - MNT: Refactor Tank's testing Assertion with CAD data. [#678](https://github.com/RocketPy-Team/RocketPy/pull/678) | |||
| ENH: Display more information in MonteCarlo prints and plots [#760](https://github.com/RocketPy-Team/RocketPy/pull/760) | |||
There was a problem hiding this comment.
Very minor, but new lines should be add to the top, not the bottom of sections. This is common confusion.
There was a problem hiding this comment.
Ah I see, it's also in the wrong section. fixed now
| for key in keys: | ||
| plt.figure() | ||
| plt.hist(self.monte_carlo.results[key]) | ||
| plt.title(f"Histogram of {key}") | ||
| plt.ylabel("Number of Occurrences") | ||
| fig, (ax1, ax2) = plt.subplots(2, 1, height_ratios=[1, 3], figsize=(8, 8)) | ||
|
|
||
| # Plot boxplot | ||
| ax1.boxplot(self.monte_carlo.results[key], vert=False) | ||
| ax1.set_xlabel(key) | ||
| ax1.set_title(f"Box Plot of {key}") | ||
|
|
||
| # Plot histogram | ||
| ax2.hist(self.monte_carlo.results[key]) | ||
| ax2.set_title(f"Histogram of {key}") | ||
| ax2.set_ylabel("Number of Occurrences") | ||
|
|
||
| plt.tight_layout() | ||
| plt.show() |
There was a problem hiding this comment.
@Lucas-Prates should we really plot the 3 plot altogether? Perhaps letting the user activate/deactivate some of them if needed?
rocketpy/simulation/monte_carlo.py
Outdated
| mean = np.mean(values) | ||
| stdev = np.std(values) | ||
| self.processed_results[result] = (mean, stdev) | ||
| pi_low = np.quantile(values, 0.025) | ||
| pi_high = np.quantile(values, 0.975) | ||
| median = np.median(values) | ||
| self.processed_results[result] = (mean, median, stdev, pi_low, pi_high) | ||
| except TypeError: | ||
| self.processed_results[result] = (None, None) | ||
| self.processed_results[result] = (None, None, None, None, None) | ||
|
|
||
| # Import methods |
There was a problem hiding this comment.
try:
mean = np.mean(values)
stdev = np.std(values)
self.processed_results[result] = (mean, stdev)
pi_low = np.quantile(values, 0.025)
pi_high = np.quantile(values, 0.975)
median = np.median(values)
except TypeError:
mean = None
stdev = None
pi_low = None
pi_high = None
median = None
self.processed_results[result] = (mean, median, stdev, pi_low, pi_high)Alternatively, we could make 5 try/except blocks instead of one single.
There was a problem hiding this comment.
Oh yeah sorry i missed that, I have committed this suggestion
I don't think 5 try/except blocks is necessary. A situation where only one of these calculations fails seems unlikely to me. ie: either they all fail or none fail so catching them all in the same makes sense.
|
Apologies for duplicate. I saw that before opening the PR but it seemed like it was unfinished and contained errors I believe this PR gets closer to resolving the original issue by combining the graphs onto the same plot Let me know anything I can do to resolve this |
No problems. Your approach looks good. Once again, thanks for your submission. |
|
@EvanMad our team is officially on vacations until January 6th. Meanwhile, what you could already do is to solve linter issues pointed out by the GitHub Actions CI, or also solving the merge conflicts error. |
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Prints maximum, minimum, mean, std dev
Plots ^ parameters with histograms
New behavior
Prints maximum, minimum, mean, std dev, 95% PI Lower, 95% PI Upper
Plots ^ parameters with histograms and boxplots
Breaking change
Additional information
For issue #730