Fix cluster solver check#2
Conversation
|
test this please |
|
Can I test this patch? |
|
Can this be merged? |
| using CovarianceDomain = dca::func::dmn_variadic<FunctionDomain, FunctionDomain>; | ||
|
|
||
| dca::func::function<double, CovarianceDomain> covariance_test("test"); | ||
| dca::func::function<double, CovarianceDomain> covariance_test2("test2"); |
There was a problem hiding this comment.
Could you please explain the difference between covariance_test and covariance_test2 and why their result is the same in the end?
There was a problem hiding this comment.
Changed the variable name and added a description. 0341b2e
There was a problem hiding this comment.
I have a suggestion: how about renaming covariance_test2 (or covariance2) to covarianceAndAvg_test (or covarianceAndAvg) to better reflect what the test is doing?
|
|
||
| private: | ||
| parameters_type& parameters; | ||
| concurrency_type& concurrency; |
There was a problem hiding this comment.
I think this slipped into the PR.
|
|
||
| for (int i = 0; i < covariance_test.size(); i++) | ||
| EXPECT_DOUBLE_EQ(covariance_expected(i), covariance_test(i)); | ||
| for (int i = 0; i < covariance.size(); i++) { |
There was a problem hiding this comment.
Maybe explain why the results are the same.
There was a problem hiding this comment.
I don't think an explanation is needed for why a method called compute A, and one called compute A and B, provide the same A.
There was a problem hiding this comment.
So do we still need to maintain computeCovariance? Do we ever call with a different estimate than the average?
There was a problem hiding this comment.
No, not really at the moment. It was written by Andrei in case we need to compute (x-y)(x-y)^t with a generic y != avg(x). But currently, there is no test that needs it.
gbalduzz
force-pushed
the
fix_cluster_solver_check
branch
2 times, most recently
from
806f212 to
b4f292e
Compare
gbalduzz
force-pushed
the
fix_cluster_solver_check
branch
from
b4f292e to
c9d5f6f
Compare
|
So I thought this whole thing had to do with, but it doesn't appear to. |
|
We apply the 1/N factor with the method sum_and_average. We need the ML estimator, not the ubiased one (with 1/N-1). |
|
Ok I see, got thrown off by the multiple calls to sum_and_average. Ok I'm fine with this being merged. |
|
test this please |
|
I'm going to implement @yingwaili's suggestions and merge it. |
|
retest this please |
|
retest this please |
|
Ready to merge if tests pass. |
Merge dist g4 master to adios2
A non-existent output.directory previously surfaced only as a cryptic HDF5 crash. Add OutputParameters::validate(), called from Parameters::readInput on first rank only, which throws std::invalid_argument with a clear message naming the missing directory. Validation is kept separate from OutputParameters::readWrite so the parsing path stays disk-free and unit-testable; this also avoids needing the existing ReadAll fixture to point at a real path. Follow-up PRs for bugs #2–#4 are expected to add validate() to other parameter sections under the same convention.
A DCA++ executable is compiled for one model and reads one "*-model"
section. Previously a typo'd or missing model section was silently
ignored: ModelParameters::readWrite swallows the missing-group exception
and the run proceeds on default model parameters.
Detect this at parse time, while the parsed tree is still live:
* Instrument the JSON reader to track which top-level groups were
read.
JSONObject gains a mutable accessed_ flag (set on successful
getGroup); JSONGroup::childGroupAccess() / JSONReader::
topLevelGroupAccess() report {name, accessed} as ChildGroupStatus.
getGroup also switched from operator[] to find(), so a lookup miss
no
longer inserts a null entry.
* Add checkModelSections (model_section_check.hpp), a standalone
helper
invoked from readInput for the JSON path. It reacts to three cases:
- typo / wrong file (model section present, none read) -> throw
std::invalid_argument;
- multi-model file (built model read, others present) -> warn;
- no model section at all -> warn, so model-agnostic uses of
Parameters keep working.
The check is compile-time guarded to JSONReader, since the HDF5
reader does not implement a model-section input guard (justified,
since HDF5 input is, presumably, machine-generated.
Tests: reader-level access tracking in json_reader_test, and unit
coverage of all checkModelSections branches in a new
model_section_check_test.
5 participants
The covariance matrix should be computed against the sample average, not the expected value, as it was done in the statistical validation test. This PR fix this issue in the cluster_solver_check application.