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

Finished Bayesian tutorial #383

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Finished Bayesian tutorial #383

wants to merge 2 commits into from

Conversation

benRenard
Copy link
Collaborator

No description provided.

Copy link
Member

@nghi-truyen nghi-truyen left a comment

Choose a reason for hiding this comment

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

Merci Ben,
Ma première remarque est que la doc prend du temps pour compiler comme il y a pas mal de fonction d'optimisation dans ce tuto. Serait-il possible de rajouter le maxiter dans termination_crit de optimize_options pour limiter le temps ça prenne pour l'optim (je pense qu'on a pas forcément besoin d'une convergence dans un tuto); et aussi si possible utiliser le Lez au lieu de la Cance pour être encore plus rapide ?

Copy link
Collaborator

@asjeb asjeb left a comment

Choose a reason for hiding this comment

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

model0 -> model_0 (Truyen m'a corrigé sur des tuto à moi)

3. it naturally handles multi-gauge calibration, with a built-in weighting of the information brought by each gauge.

This tutorial builds on the :ref:`tutorial on model calibration<user_guide.classical_uses.split_sample_temporal_validation>`: we recommend that you follow it first.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This tutorial builds on the :ref:`tutorial on model calibration<user_guide.classical_uses.split_sample_temporal_validation>`: we recommend that you follow it first.
This tutorial builds on the :ref:`<user_guide.classical_uses.fully_distributed_calibration>`: we recommend that you follow it first.

model0 = smash.Model(setup, mesh)

We finally optimize this model using the standard, non-Bayesian approach described in the :ref:`model calibration tutorial<user_guide.classical_uses.split_sample_temporal_validation>`. Note that in addition to the four model parameters ``cp``, ``ct``, ``kexc`` and ``llr``, we also calibrate the initial state values (``hp`` and ``ht``) so that we work with 'good' initial state values in the remainder of this tutorial. In addition, a single gauge is used for calibration (the most downstream one, with code "V3524010").
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
We finally optimize this model using the standard, non-Bayesian approach described in the :ref:`model calibration tutorial<user_guide.classical_uses.split_sample_temporal_validation>`. Note that in addition to the four model parameters ``cp``, ``ct``, ``kexc`` and ``llr``, we also calibrate the initial state values (``hp`` and ``ht``) so that we work with 'good' initial state values in the remainder of this tutorial. In addition, a single gauge is used for calibration (the most downstream one, with code "V3524010").
We optimize the model using the standard approach described in the :ref:`<user_guide.classical_uses.fully_distributed_calibration>`. In addition to the four model primary parameters ``cp``, ``ct``, ``kexc`` and ``llr``, we also calibrate the initial state values (``hp`` and ``ht``) so that we work with 'good' initial state values in the remainder of this tutorial. A single gauge is considered for calibration (the most downstream one, with code "V3524010").

Bayesian Estimation Approach
============================
==============================
Bayesian Estimation in `smash`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Bayesian Estimation in `smash`
Bayesian Estimation Approach

.. ipython:: python

calibrated_parameters=["cp","ct","kexc","llr","hp","ht"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
calibrated_parameters=["cp","ct","kexc","llr","hp","ht"]
calibrated_parameters = ["cp", "ct", "kexc", "llr"," hp", "ht"]

Comment on lines +39 to +42
optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges}
model0.optimize(optimize_options=optimize_options,cost_options=cost_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
calibration_gauges=["V3524010"]
optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges}
model0.optimize(optimize_options=optimize_options,cost_options=cost_options)
optimize_options={
"parameters": calibrated_parameters
}
calibration_gauges = ["V3524010"]
cost_options = {
"gauge": calibration_gaugesy
}
model_0.optimize(
optimize_options = optimize_options,
cost_options = cost_options
)

optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges}
model0.optimize(optimize_options=optimize_options,cost_options=cost_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
model0.optimize(optimize_options=optimize_options,cost_options=cost_options)
model_0.optimize(optimize_options=optimize_options,cost_options=cost_options)


optimize_options={"parameters": calibrated_parameters,"control_tfm":'keep'}
info=smash.optimize_control_info(model=model0,optimize_options=optimize_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
info=smash.optimize_control_info(model=model0,optimize_options=optimize_options)
info=smash.optimize_control_info(model=model_0,optimize_options=optimize_options)

optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges}
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
model=smash.bayesian_optimize(model=model_0,optimize_options=optimize_options,cost_options=cost_options)

optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges,"control_prior":priors}
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
model=smash.bayesian_optimize(model=model_0,optimize_options=optimize_options,cost_options=cost_options)

optimize_options={"parameters": calibrated_parameters}
cost_options={"gauge": calibration_gauges}
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
model=smash.bayesian_optimize(model=model0,optimize_options=optimize_options,cost_options=cost_options)
model=smash.bayesian_optimize(model=model_0,optimize_options=optimize_options,cost_options=cost_options)

@asjeb
Copy link
Collaborator

asjeb commented Feb 11, 2025

Je t'ai proposé des modif directement à commiter depuis le git. Mais je pense que pour la prochaine passe, je ferai du pull/push.
La compilation est longue j'aurais tendance à proposer que tu passes les cellules python en block, comme utiliser dans le contributor_guide/development_process_details.rst, ensuite tu peux référencer les images comme dans math_num/forward_structure.rst que tu mets dans le _static ; ou faire un max_iter comme suggérer par Truyen.
Convention anglo-sax pour moi avec des espaces autour des =, pas obligatoire mais ça rend le code plus lisible.
Je t'ai espacé certain de tes appels de fonctions et tes dico, dis-moi ce que tu en penses.

@benRenard
Copy link
Collaborator Author

Je ne suis pas fan de rajouter des "max_iter" pour aller plus vite, ca pollue inutilement le tuto je trouve. Je prefere soit utiliser le Lez (effectivement ca va plus vite, mais bon j'ai ecrit tout le tuto sur la base de la Cance donc c'est pas trivial), soit utiliser les cellules "block". C'est quoi un temps de compil "acceptable" ?

Pour les conventions, je vous fais confiance, il faut privilegier celles de smash.

@nghi-truyen
Copy link
Member

@benRenard , @asjeb , j'ai crée un PR #385 pour simplifier la génération des outputs lorsqu’on utilise code-block au lieu de ipython dans les tutoriels. Vous pouvez y jeter un œil, en particulier sur le fichier contributor_guide (pas besoin d’entrer dans les détails du code).

Copy link
Collaborator

@pag13 pag13 left a comment

Choose a reason for hiding this comment

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

Très intéressant ! J'ai mis des suggestions mineures de forme.

1. it allows using prior information on parameters when available;
2. it explicitly recognizes the uncertainties affecting both the model-simulated discharge (structural uncertainty) and the discharge data used to calibrate the model (observation uncertainty);
3. it naturally handles multi-gauge calibration, with a built-in weighting of the information brought by each gauge.
Copy link
Collaborator

Choose a reason for hiding this comment

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

expliciter encore "built-in", intrinsec/automatic weighting property?

model0 = smash.Model(setup, mesh)

We finally optimize this model using the standard, non-Bayesian approach described in the :ref:`model calibration tutorial<user_guide.classical_uses.split_sample_temporal_validation>`. Note that in addition to the four model parameters ``cp``, ``ct``, ``kexc`` and ``llr``, we also calibrate the initial state values (``hp`` and ``ht``) so that we work with 'good' initial state values in the remainder of this tutorial. In addition, a single gauge is used for calibration (the most downstream one, with code "V3524010").
Copy link
Collaborator

Choose a reason for hiding this comment

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

bonne idée de mq il est aussi possible d'optimiser les états

plt.legend();
plt.title(title);
return(plt)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Modifier verbose out pour les appels faits ensuite de cette fonction doplot qui produit dans la doc html générée des messages:
OUTXX: <module 'matplotlib.pyplot' from '/home/pagarambois/anaconda3/envs/smash-dev/lib/python3.12/site-packages/matplotlib/pyplot.py'>


The figure below compares the observed and the simulated discharge time series at gauge "V3517010" and indeed shows a quite poor fit, leading to a rather high uncertainty.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And unrealisticly high parameters values?
Sur la doc que j'ai compilé les incertitudes sont rather high en effet (10e10), c'est cela ?
Les paramètres obtenus sont très grandds aussi :
cp-0 estimated at : 1123919500.0000
ct-0 estimated at : 1127735353.0000
kexc-0 estimated at : 995713886.0000
llr-0 estimated at : 1110648003.0000
sg0-V3524010 estimated at : 1044549471.0000
sg0-V3515010 estimated at : 897988541.0000
sg0-V3517010 estimated at : 897988541.0000
sg1-V3524010 estimated at : 1045220557.0000
sg1-V3515010 estimated at : 1050589267.0000
sg1-V3517010 estimated at : 1052602533.0000

Ajouter un commentaire sur cela?

print(f"{info['name'][i]} estimated at : {info['x'][i]:.4f}")

The figure below compares the observed and the simulated discharge time series at gauge "V3517010" and indeed shows a quite poor fit, leading to a rather high uncertainty.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Le nom de jauge pourrait être ajouté dans le titre du graphe mais très bien en l'état déjà.

@@ -1,8 +1,204 @@
.. _user_guide.in_depth.bayesian_estimation_approach:
Copy link
Member

@nghi-truyen nghi-truyen Feb 17, 2025

Choose a reason for hiding this comment

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

Reminder/note: we can use smash.default_optimize_options to show the default optimization options..;
mention that Bayes optimize is currently not available for ANN mapping..

@asjeb asjeb linked an issue Feb 18, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a 'Bayesian estimation' case study
4 participants