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

Zoidberg changes #2219

Merged
merged 7 commits into from
Feb 9, 2021
Merged

Zoidberg changes #2219

merged 7 commits into from
Feb 9, 2021

Conversation

dschwoerer
Copy link
Contributor

Some changes to zoidberg.

I also run black (7915e1a ) - if that is not appreciated I can push without that commit ...

osa isn't using numpy, so arrays are huge in memory. Chunking the
download hugely helps reducing memory usage.
* total count was off
* local count was off
* move to end and add plus one to ensure we always end with 100%
Previously it was sometimes 99.99%, which is confusing if it crashes
afterwards.
* float is not needed, we are using python3, division defaults to
true_div
* This seems to help with numerical convergence.
@bendudson
Copy link
Contributor

Thanks @dschwoerer ! Black formatting definitely appreciated, and the changes sound good

Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

LGTM


cnt = 0
underrelax = 1
print()
Copy link
Member

Choose a reason for hiding this comment

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

Extra print() snuck in?

@ZedThree
Copy link
Member

ZedThree commented Feb 9, 2021

One test is timing out, quite annoying. Rather than rerun all the tests (github still won't let you just rerun one!), I'll merge this into 3D metrics branch and let it rerun there

@ZedThree ZedThree merged commit 4831afc into coord3d_merged2 Feb 9, 2021
@ZedThree ZedThree deleted the zoidberg_changes branch February 9, 2021 14:29
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.

None yet

3 participants