Skip to content

Minor updates: formatting, docstring updates, quality assurance, import fixes - #70

Open
astrobatty wants to merge 8 commits into
release/v1.0.1from
release/v1.0.1-fixes
Open

astrobatty wants to merge 8 commits into
release/v1.0.1from
release/v1.0.1-fixes

Conversation

@astrobatty

Copy link
Copy Markdown
Collaborator

No description provided.

@astrobatty
astrobatty requested a review from johnh2o2 October 16, 2026 21:22
@astrobatty

Copy link
Copy Markdown
Collaborator Author

I reviewed the updated files, but it looks like there are some sha256 checks prevented the tests from passing. @johnh2o2 could you take a look at this? Thanks

@johnh2o2 johnh2o2 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Left some comments / questions, thanks a ton!

Comment thread cuvarbase/bls.py
# there (see full_bls_no_sol_fused in bls_common.cuh); any other
# (noverlap, dphi) combination keeps the host-side loop, as do
# caller-provided ``functions`` dicts without the fused kernel.
fused_func = None

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Will this screw anything up?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nope. It is simply not used. The variable is defined within the try-except clause.

Comment thread cuvarbase/memory/__init__.py Outdated
which BLS/CE users never need to load.
"""

from .nfft_memory import NFFTMemory

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think the lines below lazily import these modules so things don't break when you don't have GPU (forget if there's other reasons like overhead etc.) any reason to revert to non-lazy imports?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You are right. I shouldn't do this.

@johnh2o2

Copy link
Copy Markdown
Owner

Yea will do

This branch has not been deployed

No deployments
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.

2 participants