-
Notifications
You must be signed in to change notification settings - Fork 3
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
Allow ability to extend a sweep with a prior default value #5
Conversation
Simplifies the logic in several functions by allowing chaining Nullable codepaths. Helps keep logic relatively flat instead of nested ifs. ``` if x is None: if y is None: # do a else: # do b else: # do c ``` becomes (something like) ``` outcome = ( Maybe(x) .or(y) ) ```
@@ -7,7 +7,7 @@ version_files = ["pyproject.toml"] | |||
|
|||
[tool.ruff.lint] | |||
select = ['F', 'E', 'W', 'B'] | |||
ignore = ['E501', 'E701'] | |||
ignore = ['E501', 'E701', 'B023'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new warning is for unbound variable captures in a for loop. Python really isn't a great language for functional programming.
Something like this:
obj = {}
for i in range(10):
call_later(lambda: obj['a'] = i)
depending on when that call_later
function runs (e.g. immediately, or some other time in another thread) the result of the loop might be different.
With the Maybe
monad, we use exactly this pattern. However, we know that function executions always happen synchronously and are blocking, so this is a safe pattern. So we'll just tell ruff
to ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a Maybe
monad. It's an algebraic type, meaning that transformations (e.g. via map
and otherwise
) are type-preserving --- they also return a Maybe
. The transformation may alter the interior type, i.e. Maybe[str]
might become a Maybe[int]
in the following:
maybe_str = get_user_input()
maybe_int = maybe_str.map(parseInt)
Maybe
monads are quite helpful when you have complex "nullable" logic where you might go down one of many code paths depending on if any of those code paths return a None
.
Ideally, we would refactor a lot of the internal code to use Maybe[T]
instead of T | None
for consistency and to reduce several if x is None: ...
guards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I will work on adding maybe
monad's to other parts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Consider the QRC algorithm implementation where we assume
beta=1.0
by default. We execute an experiment like:then later we decide to sweep
beta
:A problem occurs where every result is now considered invalidated, despite the fact that we already have results for the
beta=1.0
case from the first experiment execution.This PR addresses exactly this issue. The PR is structured in a sequence of 3 commits, it might be useful to go commit-by-commit (i.e. in order to see where the Maybe monad fits in).
You can test this PR with the following (run once to generate the v0 table, uncomment and generate the v1):
We expect the
beta=1.0
rows to have ids0, 1, 2, 3
and the full range of the v1 table to be[0, 11]
. The original implementation would have ids[4, 15]
.