-
-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add LOOCV config option #2153
base: develop
Are you sure you want to change the base?
Add LOOCV config option #2153
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a few hacks in here, but it is still a draft 😄
Could you please give an example output for some tutorial?
Currently, we can only switch it on or off and default is off. Maybe we want sth in between in the end(?)
@@ -75,7 +75,7 @@ struct BackendSelector { | |||
// Specialization for the RBF Eigen backend | |||
template <typename RBF> | |||
struct BackendSelector<RBFBackend::Eigen, RBF> { | |||
typedef mapping::RadialBasisFctMapping<RadialBasisFctSolver<RBF>> type; | |||
typedef mapping::RadialBasisFctMapping<RadialBasisFctSolver<RBF>, bool> type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brief docs what the bool is.
@@ -217,6 +217,8 @@ MappingConfiguration::MappingConfiguration( | |||
auto attrPolynomial = makeXMLAttribute(ATTR_POLYNOMIAL, POLYNOMIAL_SEPARATE) | |||
.setDocumentation("Toggles use of the global polynomial") | |||
.setOptions({POLYNOMIAL_ON, POLYNOMIAL_OFF, POLYNOMIAL_SEPARATE}); | |||
auto attrCrossValidation = makeXMLAttribute(ATTR_CROSS_VALIDATION, false) | |||
.setDocumentation("Computes and logs a cross-validation error using the coupling data and the mapping matrix. This can be useful for tuning the basis function. Note that this might become computationally expensive."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setDocumentation("Computes and logs a cross-validation error using the coupling data and the mapping matrix. This can be useful for tuning the basis function. Note that this might become computationally expensive."); | |
.setDocumentation("Computes and logs a cross-validation error using coupling data and the mapping matrix. This can be useful for tuning the basis function. Note that this might become computationally expensive."); |
To make clearer that the mapping matrix is static and the data dynamic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we give an estimate on the "computationally expensive"? 5x higher than the mapping without cross validation or similar?
mesh::PtrMesh inputMesh, | ||
mesh::PtrMesh outputMesh); | ||
|
||
/// Evaluates a conservative mapping and agglomerates the result in the given output data | ||
void mapConservative(const time::Sample &inData, Eigen::VectorXd &outData) const; | ||
|
||
/// Evaluates a consistent mapping and agglomerates the result in the given output data | ||
void mapConsistent(const time::Sample &inData, Eigen::VectorXd &outData) const; | ||
std::array<double, 3> mapConsistent(const time::Sample &inData, Eigen::VectorXd &outData) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "3" is the number of components, right?
So, for scalar data the last two components will remain unused?
} | ||
std::vector<double> logErrors; | ||
std::copy_n(maxErrors.begin(), inData.dataDims, std::back_inserter(logErrors)); | ||
// Do we want to log every iteration about disabled error metrics? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, why should we? I would remove this completely.
Main changes of this PR
Follow-up on #2059, which makes the change user-configurable and enables a consistent logging.
Motivation and additional information
I left out the parallel aggregation for PUM, as it would introduce unnecessary communication.
Author's checklist
pre-commit
hook to prevent dirty commits and usedpre-commit run --all
to format old commits.make changelog
if there are user-observable changes since the last release.Please check the naming of the option, the logging and the default value.
Reviewers' checklist