search

dwelter / pestpp

PEST++ inverse model project
32 stars 25 forks source link

BASE realization being dropped? #28

Open wkitlasten opened 6 years ago

wkitlasten commented 6 years ago

No entry for the "base" realization in any of my par or obs ensembles other than the initial. No mention of dropping realization "base" due to bad_phi in the rec file. Should the "base" realization be retained no matter what in order to evaluate the minimum error variance solution in the process model, or is there another way?

jtwhite79 commented 6 years ago

The base real probably should be retained but if it goes wrong it has the potential to throw off the phi distro for the ensemble.

Did you set ++ies_include_base(false) ? The base should be included by default? Or did you restart and base wasn't in The restart ensemble?

wkitlasten commented 6 years ago

I restarted using existing ensembles from a previous run (pre subset_how) as well as generating new ensembles (for a different model). Base is in .base.obs.csv, .0.obs.csv, and .0.par.csv.... but not in .1.obs.csv, and .1.par.csv, or any of the following results.

include_base is not set to false.

On Mon, Jul 9, 2018 at 1:11 PM, jtwhite79 notifications@github.com wrote:

The base real probably should be retained but if it goes wrong it has the potential to throw off the phi distro for the ensemble.

Did you set ++ies_include_base(false) ? The base should be included by default? Or did you restart and base wasn't in The restart ensemble?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dwelter/pestpp/issues/28#issuecomment-403604685, or mute the thread https://github.com/notifications/unsubscribe-auth/AOSbxNhdZvwOKx1gou8FIeRmcXuyw0b0ks5uE7jagaJpZM4VICFU .

-- Wes Kitlasten United States Geological Survey 2730 N. Deer Run Road Carson City, NV 89701 (775) 887-7711

jtwhite79 commented 6 years ago

Alright, sounds buggy. Can you send me the files?

jtwhite79 commented 6 years ago

Ok: if ++ies_num_reals(200) is used with the ++ies_par_en(file.csv) and file.csv has more than 200 reals, only the first 200 are used...so far so good. But, the way things are currently working, the base real is added to num_reals stochastic realizations so you end up with 201 realization the first time through. Then, when you restart, with ies_num_reals(200) the 201st real (base) is not used in the existing par and obs ensembles: frowny face. So, this brings up the issue: should the base realization be in addition to the stochastic reals or should it replace one of the stochastic reals?

I tested by setting ies_num_reals(201) and it seemed to carry the base real from the existing par and obs ensembles. Please let me know if that is not the issue.

wkitlasten commented 6 years ago

It seems more intuitive to have base in addition to the num_reals stochastic... but that's just like my opinion man.

I thought num_reals was ignored when existing ensembles were used. Did that change, or was I mistaken?

So leaving num_reals out should be one way to solve this issue?

On Mon, Jul 9, 2018 at 5:05 PM, jtwhite79 notifications@github.com wrote:

Ok: if ++ies_num_reals(200) is used with the ++ies_par_en(file.csv) and file.csv has more than 200 reals, only the first 200 are used...so far so good. But, the way things are currently working, the base real is added to num_reals stochastic realizations so you end up with 201 realization the first time through. Then, when you restart, with ies_num_reals(200) the 201st real (base) is not used in the existing par and obs ensembles: frowny face. So, this brings up the issue: should the base realization be in addition to the stochastic reals or should it replace one of the stochastic reals?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dwelter/pestpp/issues/28#issuecomment-403657352, or mute the thread https://github.com/notifications/unsubscribe-auth/AOSbxC4bojk_CM5Ij3aE_7YJvC_S0uNsks5uE---gaJpZM4VICFU .

-- Wes Kitlasten United States Geological Survey 2730 N. Deer Run Road Carson City, NV 89701 (775) 887-7711

jtwhite79 commented 6 years ago

++ies_num_reals is still used even with existing ensembles as a means to limit the number of reals used. Maybe that wasn't the smartest idea? I will change function so that if existing ensembles and ies_num_reals are used and ies_include_base is true, then add 1 to the num_reals counter to make sure your situation is covered - kewl?

wkitlasten commented 6 years ago

Sounds great, although now I know how num_reals works with existing ens, and knowing is half the battle.

In retrospect, it probably only worked when I was using an existing ensemble that had some dropped reals... I just didn't realize it.

On Mon, Jul 9, 2018 at 7:19 PM, jtwhite79 notifications@github.com wrote:

++ies_num_reals is still used even with existing ensembles as a means to limit the number of reals used. Maybe that wasn't the smartest idea? I will change function so that if existing ensembles and ies_num_reals are used and ies_include_base is true, then add 1 to the num_reals counter to make sure your situation is covered - kewl?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dwelter/pestpp/issues/28#issuecomment-403678044, or mute the thread https://github.com/notifications/unsubscribe-auth/AOSbxPKdDzFaJ9kuNZDQGhgA4tkORZTqks5uFA9IgaJpZM4VICFU .

-- Wes Kitlasten United States Geological Survey 2730 N. Deer Run Road Carson City, NV 89701 (775) 887-7711

jtwhite79 commented 6 years ago

Just to be clear, if you omit ++ies_num_reals, the existing ensembles will not be truncated. 4.0.3 on develop now adds one to the passed ies_num_reals if ies_include_base is True. This should help the situation...

jtwhite79 commented 6 years ago

Hey Wes - just wanted to let you know I've completely crawfished on this idea. I've decided to make the base real replace one of the stochastic reals so if you request 100 reals, you get exactly 100 reals. This should also make sure if you restart and have ies_num_reals set, the base real will be included. This function in now in 4.0.6 on develop.

I also made a small change to the subset_how functionality: if the base real is present, it will always be in the subset - I think that is prob the most important real to have a low phi. all good?

wkitlasten commented 6 years ago

Sounds great. That will be on the intel_c_windows branch too, yes?

On Mon, Jul 23, 2018 at 1:29 PM, jtwhite79 notifications@github.com wrote:

Reopened #28 https://github.com/dwelter/pestpp/issues/28.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dwelter/pestpp/issues/28#event-1748649337, or mute the thread https://github.com/notifications/unsubscribe-auth/AOSbxLU1lmCqLuyhwLqBV3lUMNDOnb9kks5uJjIVgaJpZM4VICFU .

-- Wes Kitlasten United States Geological Survey 2730 N. Deer Run Road Carson City, NV 89701 (775) 887-7711

jtwhite79 commented 6 years ago

Should be. I rebuilt those too....

wkitlasten commented 6 years ago

Looking good... and thanks to Mike's suggestion to use intel_c_windows version I am able to pick up twice as many cores on our cluster. Thanks!

On Mon, Jul 23, 2018 at 6:41 PM, jtwhite79 notifications@github.com wrote:

Should be. I rebuilt those too....

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dwelter/pestpp/issues/28#issuecomment-407252605, or mute the thread https://github.com/notifications/unsubscribe-auth/AOSbxE4CciZkaCbgbC17rNBjb59n9pmdks5uJntGgaJpZM4VICFU .

-- Wes Kitlasten United States Geological Survey 2730 N. Deer Run Road Carson City, NV 89701 (775) 887-7711