Skip to content
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 outputs for ToolboxMor and use CRBModels #1840

Merged
merged 19 commits into from
May 8, 2022

Conversation

romainhild
Copy link
Contributor

  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes?
  • Have you successfully run the Feel++ testsuite with your changes locally?
  • Have you written Doxygen comments in your contribution ?

  • Fixes Use a model for CRB applications #1797
    • add class CRBModelProperties, CRBModelParameters and CRBModelOutputs
    • ParameterSpace can be created from a CRBModelParameters
    • In the json, we need to add a section CRBParameters which contains parameters with min and max values
  • Add SensorPointwise to evaluate a field at a certain point
  • ToolboxMor uses CRBModelOutputs to create outputs from a section CRBOutputs in the json
    • Types of outputs currently available: mean, integrate, sensor, point
    • mean and integrate take markers, topodim, and optionaly expr
    • sensor takes coord and radius
    • point takes coord
    • see examples here
  • ToolboxMor uses DEIM for the outputs of type mean and integrate if the expr is dependant on the parameters.
    • the expr should use crb_u, crb_grad_u_{0,1,2} or crb_dn_u and the CRBParameters for the symbols
    • DEIM now creates its own options depending on the prefix at the construction
  • Add convergence tests for feelpp_mor_heat app on the field and outputs

For now, the outputs works only when using a scalar function space.

@romainhild
Copy link
Contributor Author

@thomas-saigre some changes here will impact you, you should check if everything is working for you.

romainhild added 4 commits April 23, 2022 15:15
skip feelpp skip toolboxes
skip feelpp skip tests skip toolboxes
@romainhild
Copy link
Contributor Author

@prudhomm @vincentchabannes can you review the PR ?
@thomas-saigre you should look at the changes in this branch, I have modified some of your tests

@thomas-saigre
Copy link
Contributor

@thomas-saigre you should look at the changes in this branch, I have modified some of your tests

so if I understand well, we define parameters in the JSON by two ways :

  1. CRBParameters, telling the range of values
  2. Parameters telling the "default value"

Maybe I will have to modify some code in what I've done in feature/pyrb on fb7104a57a20e4b842367b193d2b5507afac5650

@romainhild
Copy link
Contributor Author

Probably yes, but there is a value field in CRBModelParameter, so you could use this for the default value. But there is no toParameterValues() function in CRBModelParameters, since the value is not mandatory.
Anyway, you shouldn't use the toolbox to create a ModelParameters.

@thomas-saigre
Copy link
Contributor

as long as there is an easy way to get the default values, I'm fine with it !

@romainhild
Copy link
Contributor Author

you can do

for name, param in parameters:
    print(param.value())

But you should also modify the json so that the value is in the CRBParameters section.

}
}

std::tuple<std::vector<double>,std::vector<double>,std::vector<double>,std::vector<double>>
computeStats(std::vector<std::vector<double>> errs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const&

return std::make_tuple(min, max, mean, stdev);
}

void writeStats(std::ostream& out, std::vector<double> min, std::vector<double> mean, std::vector<double> max, std::vector<double> stdev)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const& for vector args

{
if ( !M_p.contains(key) )
{
CHECK( false ) << "invalid key " << key;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw exception

auto const& j_key = M_p.at(key);
if ( !j_key.is_string() )
{
CHECK( false ) << "key " << key << " not a string";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw exception

@prudhomm prudhomm merged commit 40e5dfa into develop May 8, 2022
@prudhomm prudhomm deleted the feature/toolboxmor-output branch May 8, 2022 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Use a model for CRB applications
4 participants