Helpers + Streamlit refactor #102
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* initial streamlit refactoring pass * cleanup and fixes * fix refiner strength * Modify params correctly * fix exception
|
I've tested streamlit and all seems to be working well there still. |
ct-777
reviewed
Aug 8, 2023
akx
reviewed
Aug 10, 2023
palp
commented
Aug 10, 2023
|
Re-tested streamlit and made some fixes, I've also reverted to a |
akx
reviewed
Aug 11, 2023
Comment on lines
243
to
248
| c[k], uc[k] = map( | ||
| lambda y: y[k][: math.prod(num_samples)].to(device), (c, uc) | ||
| lambda y: y[k][: math.prod(num_samples)].to( | ||
| device_manager.device | ||
| ), | ||
| (c, uc), | ||
| ) |
There was a problem hiding this comment.
Honestly, this .map abomination should just be
Suggested change
| c[k], uc[k] = map( | |
| lambda y: y[k][: math.prod(num_samples)].to(device), (c, uc) | |
| lambda y: y[k][: math.prod(num_samples)].to( | |
| device_manager.device | |
| ), | |
| (c, uc), | |
| ) | |
| c[k] = c[k][: math.prod(num_samples)].to(device_manager.device) | |
| uc[k] = uc[k][: math.prod(num_samples)].to(device_manager.device) |
(and as discussed (and implemented) before, num_samples being a list one needs to math.prod on doesn't make any sense, but that's probably beyond the scope of this PR.)
Comment on lines
+379
to
+381
| c[k], uc[k] = map( | ||
| lambda y: y[k][:num_samples].to(device_manager.device), (c, uc) | ||
| ) |
There was a problem hiding this comment.
Suggested change
| c[k], uc[k] = map( | |
| lambda y: y[k][:num_samples].to(device_manager.device), (c, uc) | |
| ) | |
| c[k] = c[k][:num_samples].to(device_manager.device) | |
| uc[k] = uc[k][:num_samples].to(device_manager.device) |
(funny how num_samples is an int here 😁)
|
Would be nice to get this squashed in so I could again rebase my PRs ;-) |
|
This'll probably need more fixing with the myriad of demo changes in 059d8e9 😞 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates to api/helper functions:
Txt2NoisyDiscretizationWrapperto helpers and creates a function to apply discretization wrappers conditionally.SamplingPipelineconstructor to allow passing a custom model_spec, and improve default paths behavior0.1.1