Conversation
Hrovatin
requested review from
AVHopp,
AdrianSosic and
Scienfitz
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
|
@Hrovatin please have a compiled version of this on your fork and link it here (ideally in the PR description) since this makes it way easier to review. If there is trouble with deploying things on your fork just ping me and I can help with that :) |
|
Also, can you check where you branched off for this PR? Just switched the branch to view the README on github and this seems to be quite outdated |
Hrovatin
force-pushed
the
docs/easy_access
branch
from
aa1cbcb to
e963d6b
Compare
AdrianSosic
left a comment
AdrianSosic
left a comment
There was a problem hiding this comment.
Hi @Hrovatin, thanks for taking the lead here. I'm welcoming improvements to the README 👍🏼 But we have to make sure that this is not at the cost of:
- introducing inconsistencies
- a lack of precision, i.e. softening the language in such a way that statements become inexact
To show you what I mean, I've added the hashtags #consistency and #rigor to many of my comments. This is only a first round anyway, more comments to come, but I'll now first go over the other existing open threads 🙃
There was a problem hiding this comment.
Pull request overview
This PR improves documentation accessibility by correcting a spelling error and adding new visual assets. The main changes include creating Draw.io source files for documentation diagrams and removing an unnecessary trailing newline.
- Corrected "bayesian" to "Bayesian" in userguide
- Added Draw.io diagram source files for quick start and API overview
- Added Python script for generating 3D landscape visualization
- Removed trailing newline in FAQ file
- Updated README with expanded use cases and clearer optimization workflow explanation
Reviewed changes
Copilot reviewed 6 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/userguide/userguide.md | Fixed capitalization of "Bayesian" |
| docs/graphics/landscape.py | Added script to generate 3D optimization landscape plot |
| docs/faq.md | Removed trailing blank line |
| docs/_static/quick_start.drawio | Added Draw.io source file for quick start diagram |
| docs/_static/api_overview.drawio | Added Draw.io source file for API overview diagram |
| README.md | Enhanced README with detailed use cases, workflow diagram, and clearer explanations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Hrovatin
force-pushed
the
docs/easy_access
branch
from
4d7ed3e to
581ecda
Compare
README.md
Outdated
| </li> | ||
| <li>Choose between different <b>target types</b>, including <a href="https://emdgroup.github.io/baybe/stable/userguide/targets.html#numericaltarget">numerical targets</a> (e.g., experimental outcome values) and <a href="https://emdgroup.github.io/baybe/stable/_autosummary/baybe.targets.binary.BinaryTarget.html">binary targets</a> (e.g., good/bad classification of experimental results).</li> | ||
| <li>Specify <b>how favourable individual target values</b> are (e.g., for matching to a specific value or saturation behaviour) via <a href="https://emdgroup.github.io/baybe/stable/userguide/transformations.html">target transformations</a>.</li> | ||
| <li>Optimize <b>multiple targets</b> at once (e.g., via <a href="https://emdgroup.github.io/baybe/stable/examples/Multi_Target/Multi_Target.html">Pareto optimization or desirability scalarization</a>).</li> |
There was a problem hiding this comment.
Generally point to user guide not example
There was a problem hiding this comment.
@AVHopp @Scienfitz @AdrianSosic For this link I did not find good replacement in the user guide as there is no matching section in the searchpace userguide:
Same for:
| - 📈 Comprehensive backtest, simulation and imputation utilities: Benchmark and find your best settings | ||
| - 📝 Fully typed and hypothesis-tested: Robust code base | ||
| - 🔄 All objects are fully (de-)serializable: Useful for storing results in databases or use in wrappers like APIs | ||
| <div align="center"> |
There was a problem hiding this comment.
Adjust the arrow on top of the peaks
There was a problem hiding this comment.
@AdrianSosic I resolved the image file issues. You can now adjust arrows in this file:
- File: docs/_static/complex_search_space.drawio (vscode plugin or Firefox- see below)
- Commit to github
- Open diagram in Firefox https://app.diagrams.net/?mode=github
- Export with these settings:
5. Replace this svg file: docs/_static/complex_search_space.svg (and after also in the mini-PR for images https://github.com//pull/726/changes )
| focusing on making this procedure easily accessible for real-world experiments. | ||
| Its utility was already shown in a variety of real-world experimental campaigns in both industry and academia. | ||
|
|
||
| ## 🔋 Batteries Included |
There was a problem hiding this comment.
@AdrianSosic @Scienfitz @AVHopp Added rewording in 080fb9e
4 participants
This PR tackles some points from #598, focused on #598 (comment) as agreed on the team meeting
Link to the built documentation
I still have issues with displaying images, will investigate this.