BaRatin-tools / BaRatinAGE

BaRatin Advanced Graphical Environment
GNU General Public License v3.0
4 stars 0 forks source link

empty hydraulic configs are incorreclty retrieved from project files which break files upon next saving #10

Closed JeromeLeCoz closed 1 year ago

JeromeLeCoz commented 1 year ago

Hello

I think I met a bug in modifying this bar.zip file (created with v2-1). I created a second hydraulic config, then computed the prior RC (OK), then failed to compute the posterior RC ("execution problem"). I can still compute the RC with the first config, but it corresponds to the second config! I fear something is mixed up in the files...

Ramon_Baratinage.bar.zip

JeromeLeCoz commented 1 year ago

I rebuilt the whole study from scratch using v2-2 beta0 and everything works well. I did not use the Duplicate button for configs and RCs as I did previously, so the bug might come from Duplicating the config...

Ramon.bar.zip

JeromeLeCoz commented 1 year ago

Follow-up:

I've just tried to use Duplicate in my v2-2 study and it seems to work properly. The pb may be some incompatibility between v2-1 and v2-2 files.

I've played with the RC extra plots and they all look fine. The new extra plot "Remarks" is very nice. However, in my Ramon case, the message says possible prior/post inconsistency for parameter a2, but when I look at the prior/post distributions, they don't look to be in conflict... Is the criterion too strict?

Minor detail: in the prior/post table, it would be great to have plus/minus signs instead of +/- (if easy to implement).

IvanHeriver commented 1 year ago

Hello

I think I met a bug in modifying this bar.zip file (created with v2-1). I created a second hydraulic config, then computed the prior RC (OK), then failed to compute the posterior RC ("execution problem"). I can still compute the RC with the first config, but it corresponds to the second config! I fear something is mixed up in the files...

Ramon_Baratinage.bar.zip

Thanks for the feedback. I cannot open this project file without error in BaRatinAGE v2.2. It turns out that for the second hydraulic configuration the files 0_control.txt and 1_control.txt in HydraulicConfiguration/1/which are supposed to contains the prior information for the parameters are not formatted correctly...

Question: what do you mean by "modyfing this bar.zip file"? Did you create the second control within BaRatinAGE v2.2? If so, do you have the bar.zip file prior to this modification so I can reproduce the steps which led to this bug. thanks

IvanHeriver commented 1 year ago

I rebuilt the whole study from scratch using v2-2 beta0 and everything works well. I did not use the Duplicate button for configs and RCs as I did previously, so the bug might come from Duplicating the config...

Ramon.bar.zip

I tried the duplicate feature (i.e. right click on a hydraulic configuration + duplicate) and everything worked fine for me (prior and posterior rating curve using the duplicated hydraulic config)...

Doesn't seem that the duplicate feature is broken...

IvanHeriver commented 1 year ago

I've just tried to use Duplicate in my v2-2 study and it seems to work properly. The pb may be some incompatibility between v2-1 and v2-2 files.

Difficult to say for sure. Would be great to be able to reproduce the steps that lead to this bug.

JeromeLeCoz commented 1 year ago

Yes, I have created the second control. Sorry, I only kept the modified file...

But this test seems to reproduce the bug: create a study with 2-1 with a hydraulic config with a single channel control. Then open it with 2-2, duplicate the hyd config, and change it to 2 channel controls (additive). Then, the prior RC is not computed... Here are the 2-1 and 2-2 files.

test.bar.zip test2-2.bar.zip

IvanHeriver commented 1 year ago

Thank you Jérôme, I was able to reproduce the bug. It is not related to the version 2.1 as it occurs also with version 2.2.

I suspect the bug to be related to duplicating an empty hydraulic configuration (i.e. nothing specified for the parameters).

I think this isn't trivial to fix and more investigation is required to know for sure the source of the problem... Is it worth fixing for v2.2?

JeromeLeCoz commented 1 year ago

I think it would be enough in 2-2 to prevent the user from doing what does not work (perhaps with a simple warning explaining that duplicating an empty hydraulic configuration is not allowed?).

IvanHeriver commented 1 year ago

I think I corrected the bug. It actually affected much more than duplication of hydraulic configurations.

A more detailed description of the bug would be:

In other words, duplicating the hydraulic configuration had not much to do with the problem.

Hopefully, the modifications I made fixed the bug.

JeromeLeCoz commented 1 year ago

Great, thx!

JeromeLeCoz commented 1 year ago

It seems the bug is still there. I repeated the same actions (using 2-1-1 and 2-2) and now get an error message when trying to compute the prior RC: image

Here's the file: test.bar.zip

JeromeLeCoz commented 1 year ago

Je précise d'ailleurs que les contrôles ne sont pas vides mais spécifiés, avec CT prior calculées.

IvanHeriver commented 1 year ago

I was not able to reproduce the bug.

I tried:

What did I miss?

JeromeLeCoz commented 1 year ago

I think my problem is different (and worse): using 2-2 beta1, I cannot run prior RC if I define juste 1 control (channel or weir)...

JeromeLeCoz commented 1 year ago

I confirm, and I don't have this problem with 2-2 beta0...

IvanHeriver commented 1 year ago

Sorry, forgot to include the BaRatin exe in the beta version I sent you... Should be fixed now. And this should be unrelated to the issue discussed here.

JeromeLeCoz commented 1 year ago

Thx, bug fixed. The 2-2 version is fully ok for me.