Restore Kronecker discrepancy shapes when replications are omitted - #633
sou-cheng-choi wants to merge 81 commits into
Conversation
…into kronecker_search
…e rule search method
…Software into lattice_kronecker
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
- Kronecker search wssd now works for other kernels, added unit test case - Lattice search doctest now checks invariant properties - Lattice.wssd can now handle coord_weights longer than the dimension - Lattice search minimum n_max value corrected - Removed user interactive prompt in Kronecker search - Fixed docstring formatting - Updated demo notebook with more interesting Kronecker comparison
* Min Python version >= 3.9 for users & >= 3.10 for developers (#606) * Min Python version * Pass Python version to environment * Pre-release tests * Get more info * Security Fix * requires-python = ">= 3.9" in pyproject.toml * Python 3.9 Compatibility * Fix versions * Fix windows test failures * Installs the missing l3backend MiKTeX package * Update demo notebook dependencies and initialization cells (#531) * Update demo notebook initialization cells * Notebooks have been moved * Add Colab dependency install cells * Fix Colab notebook install cells * Fix spelling in Colab notebooks * Remove unused os import from Colab cells * Use capture for Colab install output * Automating Colab/notebook consistency * Update * Update makefile * Add make target for Colab bootstrap classification * Debugged `harden_colab_notebook.py` and add unit tests for badge handling * Add branch notebook execution script * Skip Colab bootstrap during branch notebook execution * Download Dakota Genz points in notebook * Remove Dakota Genz generation fallback * Use gdown for Dakota Genz data download * Add local notebook execution script * Comment out gdown in Dakota notebook * Fix Windows notebook CI LaTeX setup * Address Colab readiness review feedback * Reconcile Colab tooling with develop * Refresh safe Colab bootstrap cells * Preserve notebook serialization during hardening * Better Colab notebook handling: more dependencies support and improved smoke tests * Fix CodeQL check errors * Add title to notebook and update kernel name and version * Fix errors after manually testing in Google Colab * Fix problems after manually running in Google Colab * Minor enhancements * Fix colab error * Fix typo * Simplify Colab smoke tests: drop dead PR scoping, fix diagnostics * Add harden_colab_notebook to format target * Add tools to open in Colab (for developers) * Attempt to fix Colab errors * Correct Colab notebook manifest and build process * Fix unit test failure --------- Co-authored-by: sou-cheng-choi <[email protected]> * Remove unreferenced file * Add notebook to documentation * Remove trailing spaces * Make LVS deterministic * Optimized k_const computations. Reduce memory usage. Add custom kernel input to wssd method. * Resolved UnboundLocalError when calculating WSSD for d_max = 1. Improved warning for missing sympy. Better format. * Add unit tests * Change ParameterWarngin to UserWarning * Fix GitHub Action failures * Another attempt to fix * make harden_colab_notebook * Fix windows test failure --------- Co-authored-by: Joshua Herman <[email protected]>
This reverts commit 17702a3.
Resolve notebook, Colab, CI, build, and Kronecker conflicts while preserving generator-search behavior. Align branch tests and API annotations with develop's validation rules. Validation: 699 unit tests and 275 subtests; focused numerical doctests; Lattice/Kronecker and GBM notebooks; strict Colab and test-style checks; documentation baseline gate; focused MkDocs build.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| try: | ||
| return _tg_radius_graph(x, r=r, batch=batch, loop=loop) | ||
| except (ImportError, AttributeError, RuntimeError, OSError): | ||
| _tg_radius_graph_ok = False # backend missing/broken -- use native from now on |
There was a problem hiding this comment.
Is this from LDData? If so, we should use our API to fetch it. Otherwise, it should be added to the manifest in pyproject.toml. I'll leave reviews to @fjhickernell and @AndersPride and @algo-hawk.
|
Independently re-verified this fix in a scratch worktree against Confirmed:
One remaining failure, Good to merge from my side. |
JiangruiKang
left a comment
JiangruiKang
left a comment
There was a problem hiding this comment.
I verified that the #629 shape regression is fixed, and the focused test file passes (11 tests). I am requesting changes because Lattice.expected_squared_periodic_discrepancies() flattens the replication axis and wssd() silently returns the first generating vector’s result. A two-vector reproduction gives different WSSD values, 0.75099 and 0.72815, while the replicated call returns only 0.75099. Please preserve or explicitly define the replicated result and add a regression test.
lattice_vector_wssd_search() also changes the process-wide NumPy error settings without restoring them. Please use a local error-state context. Finally, please clarify the PR’s broader search-method and MPMC scope; the description currently presents this as only a shape fix. The MPMC fallback also appears to omit PyG’s default 32-neighbor limit, which needs a test or an explanation of the intended change.
Summary
Fixes #629. On
lattice_kronecker, discrepancy helpers retained a singleton generating-vector axis even whenreplications=None. The patch removes that axis only for omitted replications and documents the resulting shapes. The regression test checks default and explicit-one-replication behavior against a direct periodic-kernel calculation.pytest test/test_dd_lattice_kronecker.py -vAI Assistance
I used my QMCPy Development Copilot to come up with the fix.
replications=Nonevs.replications=1) against the current checkout rather than trusting the issue's or the diff's claims at face value.Checklist
Fixes #629above.test_kron_disc_wssdextended with shape assertions for both the default andreplications=1cases; two docstrings (periodic_discrepancy,wssd_discrepancy) updated to document both shapes._direct_disc's pairwise definition, not just asserted.