-
Notifications
You must be signed in to change notification settings - Fork 12
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
Write fluxes #199
Write fluxes #199
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.
Nice job!
This is only comments from a first reading of the code, without testing in details on a case.
["pn", "en", "pr", "perc", "lexc", "prr", "prd", "qr", "qd"], # % gr5 | ||
["ei", "pn", "en", "pr", "perc", "prr", "qr"], # % grd | ||
["ei", "pn", "en", "pr", "perc", "prr", "prd", "qr", "qd"], # % loieau | ||
["pn", "en", "qr", "qb"], # % vic3l |
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.
for vic, add vertical drainage fluxes between each production layer (d_{umsl}, medium and bottom layers also..)
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.
(not critical, this can be fixed later)
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.
All drainage fluxes are on active cells...
SIMULATION_RETURN_OPTIONS_TIME_STEP_KEYS, | ||
STRUCTURE_RR_STATES, |
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.
if "STRUCTURE" dependence mentioned (which I agree) in this "keyword" for RR_STATES, then idem for internal fluxes? INTARNAL_FLUXES => STRUCTURE_INTERNAL_FLUXES ?
real(sp) :: m | ||
|
||
fx = returns%stats%internal_fluxes(:, :, idx) | ||
!$AD start-exclude |
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.
seen. needed here if call to the function is excluded? entire stats module has to be differentiated?
@@ -137,6 +137,7 @@ module mwd_setup | |||
integer :: nrrs = -99 | |||
integer :: nsep_mu = -99 | |||
integer :: nsep_sigma = -99 | |||
integer :: nfx = -99 |
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.
what is nfx? clarify in routine header (cf. nqz!)
@@ -366,7 +379,17 @@ subroutine simulation_checkpoint(setup, mesh, input_data, parameters, output, op | |||
rr_parameters_inc = rr_parameters_inc + 1 | |||
|
|||
end select | |||
|
|||
!$AD start-exclude |
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.
ok. to be investigated later.
@@ -202,6 +204,30 @@ subroutine gr4_time_step(setup, mesh, input_data, options, time_step, ac_mlt, ac | |||
! Transform from mm/dt to m3/s | |||
ac_qt(k) = ac_qt(k)*1e-3_sp*mesh%dx(row, col)*mesh%dy(row, col)/setup%dt | |||
|
|||
!$AD start-exclude |
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.
ok. to be investigated later.
if (returns%pn_flag) returns%pn(row, col, time_step) = pn | ||
if (returns%en_flag) returns%en(row, col, time_step) = en | ||
if (returns%pr_flag) returns%pr(row, col, time_step) = pr | ||
if (returns%perc_flag) returns%perc(row, col, time_step) = perc | ||
if (returns%lexc_flag) returns%lexc(row, col, time_step) = l | ||
if (returns%prr_flag) returns%prr(row, col, time_step) = prr | ||
if (returns%prd_flag) returns%prd(row, col, time_step) = prd | ||
if (returns%qr_flag) returns%qr(row, col, time_step) = qr | ||
if (returns%qd_flag) returns%qd(row, col, time_step) = qd |
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.
ok for this first version. flux maps getter implemented?
Wouldn't this be better to use sthg like returns%internal_fluxes_maps(:,:,:,:)
with 1..N fluxes which names is defined for each model structure (as for params or for stats%internal_fluxes above) in _constants.py?
if (returns%stats_flag) then | ||
returns%stats%internal_fluxes(row, col, 1) = pn | ||
returns%stats%internal_fluxes(row, col, 2) = en | ||
returns%stats%internal_fluxes(row, col, 3) = qr | ||
returns%stats%internal_fluxes(row, col, 4) = qb | ||
end if | ||
if (returns%pn_flag) returns%pn(row, col, time_step) = pn | ||
if (returns%en_flag) returns%en(row, col, time_step) = en | ||
if (returns%qr_flag) returns%qr(row, col, time_step) = qr | ||
if (returns%qb_flag) returns%qb(row, col, time_step) = qb | ||
!$AD end-exclude |
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.
vertical fluxes between production store layers are missing
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.
A supprimer
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.
A supprimer
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.
Je trouve ca un peu complexe de demander à l'utilisateur de cocher chacun des flux un par un. Je regrouperais bien tous les flux dans une seule variable: internal_fluxes
. L'utilisateur récupère tout mais pas grave c'est consistent avec les états.
Pour stats
, je trouve pas ca hyper clair comme mot clef. Je mettrais plutot internal_fluxes_stats
On pourrait aussi récupérer ces valeurs en sortie d'une optimisation (c'est à dire, écrire les flux sur le dernier run forward de l'optimisation) ? Ca nous oblige à relancer un run direct en fin d'optimisation si on veut récupérer ces flux
INTERNAL_FLUXES = dict( | ||
zip( | ||
HYDROLOGICAL_MODULE, | ||
[ | ||
["pn", "en", "pr", "perc", "lexc", "prr", "prd", "qr", "qd"], # % gr4 | ||
["pn", "en", "pr", "perc", "lexc", "prr", "prd", "qr", "qd"], # % gr5 | ||
["ei", "pn", "en", "pr", "perc", "prr", "qr"], # % grd | ||
["ei", "pn", "en", "pr", "perc", "prr", "prd", "qr", "qd"], # % loieau | ||
["pn", "en", "qr", "qb"], # % vic3l | ||
], | ||
) | ||
) | ||
|
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.
La par contre, on doit déclarer le nom de chaque flux pas de pb. Cependant, on restreint les flux au module hydrologique. On pourrait avoir des flux internes dans le module neige et le module de routage. Il faudrait revoir cette constante pour quelle englobe les 3 modules et donc avoir des flux internes par structure, un peu comme les parameters et états. (voir get_structure_rr_parameters, etc)
], | ||
) | ||
) | ||
|
||
SIMULATION_RETURN_OPTIONS_TIME_STEP_KEYS = ["rr_states", "q_domain"] |
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.
J'aimerais bien mettre les flux internes au moins les grilles dans les variables qui peuvent être récupérées qu'a certains pas de temps. Pour les statisques sur les flux internes on peut renvoyer toute la chronique dans l'absolu
!$AD start-exclude | ||
!internal fluxes | ||
if (returns%stats_flag) then | ||
returns%stats%internal_fluxes(row, col, 1) = ei | ||
returns%stats%internal_fluxes(row, col, 2) = pn | ||
returns%stats%internal_fluxes(row, col, 3) = en | ||
returns%stats%internal_fluxes(row, col, 4) = pr | ||
returns%stats%internal_fluxes(row, col, 5) = perc | ||
returns%stats%internal_fluxes(row, col, 6) = prr | ||
returns%stats%internal_fluxes(row, col, 7) = prd | ||
returns%stats%internal_fluxes(row, col, 8) = qr | ||
returns%stats%internal_fluxes(row, col, 9) = qd | ||
end if | ||
if (returns%ei_flag) returns%ei(row, col, time_step) = ei | ||
if (returns%pn_flag) returns%pn(row, col, time_step) = pn | ||
if (returns%en_flag) returns%en(row, col, time_step) = en | ||
if (returns%pr_flag) returns%pr(row, col, time_step) = pr | ||
if (returns%perc_flag) returns%perc(row, col, time_step) = perc | ||
if (returns%prr_flag) returns%prr(row, col, time_step) = prr | ||
if (returns%prd_flag) returns%prd(row, col, time_step) = prd | ||
if (returns%qr_flag) returns%qr(row, col, time_step) = qr | ||
if (returns%qd_flag) returns%qd(row, col, time_step) = qd | ||
!$AD end-exclude |
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.
A revoir si on change les flags de return options
!$AD start-exclude | ||
!internal fluxes | ||
if (returns%stats_flag) then | ||
returns%stats%internal_fluxes(row, col, 1) = pn | ||
returns%stats%internal_fluxes(row, col, 2) = en | ||
returns%stats%internal_fluxes(row, col, 3) = qr | ||
returns%stats%internal_fluxes(row, col, 4) = qb | ||
end if | ||
if (returns%pn_flag) returns%pn(row, col, time_step) = pn | ||
if (returns%en_flag) returns%en(row, col, time_step) = en | ||
if (returns%qr_flag) returns%qr(row, col, time_step) = qr | ||
if (returns%qb_flag) returns%qb(row, col, time_step) = qb | ||
!$AD end-exclude |
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.
A revoir si on change les flags de return options
@@ -74,6 +74,7 @@ module mwd_parameters_manipulation | |||
use mwd_options !% only: OptionsDT | |||
use mwd_returns !% only: ReturnsDT | |||
use mwd_control !% only: ControlDT_initialise, ControlDT_finalise | |||
use mwd_stats !% only: StatsDT |
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.
A enlever surement
if wrap_returns.stats_flag: | ||
wrap_returns.stats.fluxes_keys = INTERNAL_FLUXES[model.setup.hydrological_module] | ||
wrap_returns.stats.rr_states_keys = STRUCTURE_RR_STATES[model.setup.structure] | ||
|
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.
A revoir peut etre je sais pas
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.
le déplacer apres wrap_forward_run apres if ret
:
if (returns%stats_flag) then | ||
do i = 1, setup%nfx | ||
call compute_fluxes_stats(mesh, t, i, returns) | ||
end do | ||
|
||
do i = 1, setup%nrrs | ||
call compute_states_stats(mesh, output, t, i, returns) | ||
end do | ||
end if | ||
!$AD end-exclude |
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.
Peut etre mettre ca dans store_time_step
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.
Est que les nombreux commentaires et suggestions de FC ont été addressés.
Quid du mwd_stat ?
Quid de la resolution de pb de la diff auto ?
à investiguer ensemble et finaliser asap svp.
Maintenant que l'on a les flux internes est ce que une classe stat python ne serait pas tout aussi adapté ? Ou bien qq chose à récupérer sur les gradients ? |
Coucou, je ne suis pas sûr de bien comprendre, peux tu clarifier stp ? |
merge write_fluxes into main
writes fluxes :
["pn", "en", "pr", "perc", "lexc", "prr", "prd", "qr", "qd"], # % gr4
["pn", "en", "pr", "perc", "lexc", "prr", "prd", "qr", "qd"], # % gr5
["ei", "pn", "en", "pr", "perc", "prr", "qr"], # % grd
["ei", "pn", "en", "pr", "perc", "prr", "prd", "qr", "qd"], # % loieau
["pn", "en", "qr", "qb"], # % vic3l
writes states :
["hi", "hp", "ht"], # % gr4
["hi", "hp", "ht"], # % gr5
["hp", "ht"], # % grd
["ha", "hc"], # % loieau
["hcl", "husl", "hmsl", "hbsl"], # % vic3l
writes stats :
mean, var, minimum, maximum, median
when run_forward is worked with activated return_options