As discussed in PR #16, we know the low-rank candidate's rank, therefore merging two_part_factor with two_part_factor_known_rank to reduce repeated code.
Closes #17 .
Type of change
[ ] Bug fix (non-breaking change which fixes an unintended
or erroneous behavior)
[ ] New features (non-breaking change that adds functionality)
[ ] Breaking change (fix or feature that changes how existing
functionality works, other than correcting errors)
[x] Code improvement (no new functionality, but improving project structure)
[ ] Documentation only (changes that improve or expand instructions
to users, without changing program behavior)
Motivation and Context
The change avoids code repetition and matrix rank computation. It merges functions two_part_factor and two_part_factor_known_rank into a single function two_part_factor with optional argument rank. If rank is not supplied, it will be detected using numpy.linalg.matrix_rank. For the current use cases, we always now the target rank in advance. Currently, the function is used twofold:
Instantiating candidate_factor_W0 and candidate_factor_H0 in kernel Momentum3BlockModelFreeKernel if argument low_rank_candidate_L was supplied but either (or both) W0 and H0 were omitted
Factorizing the result low_rank_candidate_L into W and H for algorithms that return only L (currently: base model-free, gaussian model with single variance parameter and gaussian model with rowwise variance parameter). This is done in kernel methods self.report()
Description
This PR introduces the following changes:
Alter two_part_factor to accept optional parameter rank
two_part_factor now has an optional integer parameter rank
If rank is None (the default) it is computed using numpy.linalg.matrix_rank
As discussed in PR #16, we know the low-rank candidate's rank, therefore merging
two_part_factorwithtwo_part_factor_known_rankto reduce repeated code.Closes #17 .
Type of change
Motivation and Context
The change avoids code repetition and matrix rank computation. It merges functions
two_part_factorandtwo_part_factor_known_rankinto a single functiontwo_part_factorwith optional argumentrank. If rank is not supplied, it will be detected usingnumpy.linalg.matrix_rank. For the current use cases, we always now the target rank in advance. Currently, the function is used twofold:candidate_factor_W0andcandidate_factor_H0in kernelMomentum3BlockModelFreeKernelif argumentlow_rank_candidate_Lwas supplied but either (or both)W0andH0were omittedlow_rank_candidate_LintoWandHfor algorithms that return onlyL(currently: base model-free, gaussian model with single variance parameter and gaussian model with rowwise variance parameter). This is done in kernel methodsself.report()Description
This PR introduces the following changes:
two_part_factorto accept optional parameterranktwo_part_factornow has an optional integer parameterrankNone(the default) it is computed usingnumpy.linalg.matrix_ranktwo_part_factor_known_ranksrc/fi_nomad/util/decomposition_util.py,src/fi_nomad/util/__init__.pykernel.target_rankas second argument totwo_part_factorsrc/fi_nomad/kernels/base_model_free.py,src/fi_nomad/kernels/single_variance_gauss_model.py,src/fi_nomad/kernels/rowwise_variance_gauss_model.py,src/fi_nomad/kernels/momentum_three_block_model_free.pyTesting
test_two_part_factorinto:test_two_part_factor_rank_unknown: where the target rank is determined bynumpy.linalg.matrix_ranktest_two_part_factor_rank_known: whererankis supplied as an argument, and check thatnumpy.linalg.matrix_rankis not calledtest/test_util/test_decomposition_util.pytest_base_model_free_kernel_final_report:kernel.target_rank = 1as no kernel step has been made and the low rank candidate L is initialized asnp.ones(...), thus having rank 1test/test_kernels/test_base_model_free_kernel.pyChecklist