QMC code restructuring #433
Conversation
removing extra file not needed.
Codecov Report
@@ Coverage Diff @@
## master #433 +/- ##
==========================================
- Coverage 62.27% 61.55% -0.73%
==========================================
Files 63 79 +16
Lines 3632 4789 +1157
==========================================
+ Hits 2262 2948 +686
- Misses 1370 1841 +471
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Looking good so far! BTW you have access to the QuEraComputing repo for Bloqade so you can push your own branches with |
| First[st] = v + num_sites | ||
| end | ||
|
|
||
| @simd for i in 1:num_legs |
There was a problem hiding this comment.
Can we check if the simd is actually useful here? By measuring the time - simd optimization is not very reliable so it'd be great to check that while developing
| end | ||
| end | ||
|
|
||
| if qmc_state isa BinaryGroundState |
There was a problem hiding this comment.
I think this if else is an easy abstraction to separate out using multiple dispatch.
| ############################################################################# | ||
|
|
||
|
|
||
| @inline function _map_back_operator_list!(ocount::Int, qmc_state::BinaryQMCState, H::AbstractIsing, d::Diagnostics) |
There was a problem hiding this comment.
I think we would just remove the beginning underscore from the name.
| in_cluster[leg] = ccount # add the new leg and flip it | ||
| push!(current_cluster, leg) | ||
| if qmc_state isa BinaryGroundState | ||
| lnA += trialstate_weight_change(qmc_state, lsize, Ns, i) | ||
| end | ||
| a = Associates[leg] | ||
|
|
||
| a == 0 && continue | ||
| # from this point on, we know we're on a bond op | ||
| op = operator_list[op_indices[leg]] | ||
| w = getweightindex(H, op) - getoperatortype(H, op) | ||
| preflip_bond_type, postflip_bond_type = update_kernel!(qmc_state, H, ccount, leg, a) | ||
| lnA += ( | ||
| getlogweight(H.op_sampler, w + postflip_bond_type) | ||
| - getlogweight(H.op_sampler, w + preflip_bond_type) | ||
| ) |
There was a problem hiding this comment.
I'd separate this to a function (at least to start with) it's always beneficial to wrap a loop kernel into a function
| ocount = _map_back_basis_states!(rng, lsize, qmc_state, H) | ||
| _map_back_operator_list!(ocount, qmc_state, H, d) | ||
|
|
||
| return lsize |
There was a problem hiding this comment.
It probably worth thinking if this return value can be a state object - it would be nice the MC iteration can roughly follow an iterator interface in Julia. But this is not a blocking comment.
There was a problem hiding this comment.
Also it seems lsize is the same as input here? If I'm not missing something what's the point of returning the same value?
|
|
||
| if !(runstats isa NoStats) | ||
| fit!(runstats, :diag_update_fails, failures/count) | ||
| end |
There was a problem hiding this comment.
Can we be explicit on what return value is of this function?
| resize!(qmc_state.leg_sites, len) | ||
| resize!(qmc_state.op_indices, len) | ||
| resize!(qmc_state.in_cluster, len) | ||
| end |
There was a problem hiding this comment.
The return value should be the state not the vector
| # Now, add the 2M operators to the linked list. Each has either 2 or 4 legs | ||
| @inbounds for (n, op) in enumerate(qmc_state.operator_list) | ||
| if issiteoperator(H, op) | ||
| site = getsite(H, op) |
There was a problem hiding this comment.
It would be nice if we can seperate out the code block for each different operator (site and bond)
|
There are some general name conventions are not consistent with what we use in Bloqade. So it would be nice if you could just rename them when you spot them (so we will just check them in the end when the development is almost done). In general, we only use Camel case for types and all other things should be in snake case. And we don't usually use starting underscore to denote private function (unless you really having a hard time figuring out a proper name) |
|
Haven't addressed comments above yet but the ltfim test is running now with restructured update rules :) |
|
Suggestion for overall code structure:
The only top-level file left (apart from BloqadeQMC.jl) is still measurements.jl. Here, we need to discuss how many of the functions we really need for Bloqade. Will probably shrink significantly. |
|
Also important: run a Rydberg QMC. Currently, the QMC test uses a LTFIM. Code should be pretty much the same. And we can use the ED values from exact_diag.jl for comparison. |
|
Note: Expansion order fluctuates in finite-T-case but stays fixed for zero-T. |
Got started on this in new PR |
Will leave this unchanged until we get properly started on user interface |
|
@Atomyka Can I close this PR? |
Streamlining and restructuring QMC code.