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

Add Explanation of some Utilities to User Guide #392

Merged
merged 14 commits into from
Oct 11, 2024
Merged

Add Explanation of some Utilities to User Guide #392

merged 14 commits into from
Oct 11, 2024

Conversation

Scienfitz
Copy link
Collaborator

@Scienfitz Scienfitz commented Oct 4, 2024

Adds explanations for these utilities:

  • estimate discrete space memory size
  • control reproducibility via random seeds
  • adding fake targets and parameter noise

Note: This required some adjustment of the code execution test. It seems that list comprehensions called via exec have some weird scope, causing NameError for NumericalDiscreteParameter even though it was imported int he code block. So I am now providing a custom dict as globals and locals arg to exec which seems to unify the scopes. As far as I know this should still maintain isolation between separately tested code blocks.

@Scienfitz Scienfitz added the documentation Improvements or additions to documentation label Oct 4, 2024
@Scienfitz Scienfitz self-assigned this Oct 4, 2024
@Scienfitz Scienfitz force-pushed the docs/utils branch 7 times, most recently from 501662b to 74c34d1 Compare October 9, 2024 14:01
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Scienfitz, thanks for writing. I'll push one more suggestion to generalize the section on effective search space creation.

CHANGELOG.md Outdated Show resolved Hide resolved
tests/docs/test_docs.py Show resolved Hide resolved
tests/docs/test_docs.py Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
docs/userguide/utils.md Show resolved Hide resolved
docs/userguide/utils.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks for incorporating the changes 🥇

Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mucho like, just some minor comments. Feel free to resolve how you see fit.

baybe/utils/dataframe.py Outdated Show resolved Hide resolved
docs/userguide/utils.md Show resolved Hide resolved
docs/userguide/utils.md Show resolved Hide resolved
docs/userguide/utils.md Show resolved Hide resolved
@Scienfitz Scienfitz merged commit 9dcf27e into main Oct 11, 2024
11 checks passed
@Scienfitz Scienfitz deleted the docs/utils branch October 11, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants