Skip to content

Update to process_result in optimizer.py#37

Merged
langdal merged 1 commit intomainfrom
36-show-confidence-interval
Apr 20, 2022
Merged

Update to process_result in optimizer.py#37
langdal merged 1 commit intomainfrom
36-show-confidence-interval

Conversation

@abbl-DTI
Copy link
Copy Markdown
Member

The default call to plot_objective has been updated to include the call for confidence intervals, line 182 in the new version.
Additionally the file has been formatted with 'black'.

@abbl-DTI abbl-DTI requested a review from langdal April 20, 2022 07:21
@abbl-DTI abbl-DTI linked an issue Apr 20, 2022 that may be closed by this pull request
@langdal
Copy link
Copy Markdown
Member

langdal commented Apr 20, 2022

Does not work for me with the attached experimen
78167d7f-081c-4193-a7df-507144908bdc.json.zip
t

@langdal
Copy link
Copy Markdown
Member

langdal commented Apr 20, 2022

@abbl-DTI After a bit more testing it does now work for me on the linked experiment. I can't figure out what made it fail for some runs.

@abbl-DTI
Copy link
Copy Markdown
Member Author

@langdal Is it working as the branch is now? I think I managed to do soft reset of the branch in order to not incorporate the formatting performed by black. But the change in the call to plot_objective was also rolled back.
The check is if there are red bands around the objective plot.
I'll push the updated call to plot_objective in a short will.

@langdal
Copy link
Copy Markdown
Member

langdal commented Apr 20, 2022

The current version works yes. A side note - the change log file should never be modified manually. All changes will be overwritten next time the generator is run (which is done with each versioned release).
I will squash this PR into only the one commit containing the change to optimizer.py and then approve and merge it, ok? @abbl-DTI

@abbl-DTI
Copy link
Copy Markdown
Member Author

Sounds great.
Regarding the Changelog.md, it was updated using the procedure at the end of the README.md. It was unclear to me, what the propper procesure was for just commiting small changes, if this step was necessary.

@langdal
Copy link
Copy Markdown
Member

langdal commented Apr 20, 2022

There is no need to update the changelog on each PR - at some point it will be integrated into the automatic build process and until then it is updated with each manual release.

@langdal langdal force-pushed the 36-show-confidence-interval branch from 802ee6c to e7af3e9 Compare April 20, 2022 11:09
@langdal langdal merged commit 4fc0c1c into main Apr 20, 2022
@langdal langdal deleted the 36-show-confidence-interval branch April 20, 2022 11:10
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.

Show confidence interval

2 participants