duncan-r / SHIP

Simple Hydraulic model Interface for Python
MIT License
7 stars 6 forks source link

Full support for scenario/event/define/start-end logic #21

Open duncan-r opened 7 years ago

duncan-r commented 7 years ago

@JamesRamm I thought I'd start a separate issue for this so we can keep on top of the progress separately from the general updates.

There is quite good coverage of this in the existing library, although some are not well catered for. There can be a lot of different types of "DEFINE" block that represent different things. For instance: DEFINE Event DEFINE OutputZone

To complicate this some stuff like Event can be added using DEFINE, as above, or using if-else logic: IF EVENT == .

There are also domain specific clauses (these are not currently supported): START 1D Domain START 2D Domain

A lot of this can be a little problematic, because certain combinations of words will change the meaning. They will probably need separate rules (regex's) under the new system and probably an easy way to add in additional ones as they tend to get added fairly often (I think DEFINE Output Zone was only added a year or so ago).

The slides from this talk by Bill covers a lot of it quite well, but it's not complete and there have been some updates since this was made. https://www.tuflow.com/Download/Presentations/2012/2012%20Aust%20Workshops%20-%20TUFLOW%20Multiple%20Events%20and%20Scenarios.pdf

It may be worth while contacting TUFLOW to try and get a complete list of these?

JamesRamm commented 7 years ago

@duncan-r So at the moment in the new stuff, this will be happily parsed like any command. So, you would end up with a DEFINE node (with no command/params) with a number of children. The first child would be the Event == Something Same for OutputZone and IF blocks. So it shouldnt have a problem capturing the commands - is there further processing/recognition ship can currently do for these scenarios?

duncan-r commented 7 years ago

is there further processing/recognition ship can currently do for these scenarios?

In terms of loading? No it pretty much does that. Although it will break down any piped arguments into the event / scenario values (IF SCENARIO == one | two | three).

In terms of the API you can add/remove parts from within the logic and change the order of stuff, etc. But I don't think that's what you mean here?

Note that these can also be nested:

IF SCENARIO == one | two
    IF SCENARIO == three
        DEFINE EVENT ...
        END DEFINE
    END IF
END IF

I've also opened this to not just track progress for the changes against the existing functionality, but as a place to note anything that is not currently supported and whether to support/not support it for the moment or in the future.

JamesRamm commented 7 years ago

Ahh yes...If you have an example of a TCF file with nesting like that it could be useful. In theory the parser should handle it, but 99% of the time there is something I didn't think of..

Re-ordering is an interesting one - while possible in the new setup, its very hands-on. It is basically up to you to re-order the children list of a node how you want. I don't know what a desired API might look like for that, but it would probably be useful to offer some help....

duncan-r commented 7 years ago

but 99% of the time there is something I didn't think of..

👍 That sums up my entire experience of developing this perfectly!

There are some examples of this in the integration test models. Specifically, there are nested scenarios here: https://github.com/duncan-r/SHIP/blob/develop/integration_tests/test_data/model1/tuflow/model/test_tgc1.tgc

Which is called from this tcf. Which also has zone output zones and events assigned. https://github.com/duncan-r/SHIP/blob/develop/integration_tests/test_data/model1/tuflow/runs/test_run1.tcf

Note as well that at the top of the .tcf there are;

! Model Scenario and event values
Model Events == evnt1 | evnt3 | evnt7
Model Scenarios == scen1 | scen2 | scen6

BC Event Text == Q100
BC Event Source == Q100 | SHIP
BC Event Name == SHIP

These are event and scenario's assigned within the control files. If matching ones are passed in by the caller they will override them. I can't quite remember the exact logic of all this off the top of my head, but I have some notes, so I'll look them up.

duncan-r commented 7 years ago

Re-ordering is an interesting one

Yes. It's tricky. I think currently the library allows you to hand in a part and specify a part to put it after/before. This is a bit of an edge case and not done very often (I can't think of anytime I've actually done it, so it might not be a huge issue for the moment). As it is possible, albeit difficult, maybe we can consider it when the more useful bits are finished?

JamesRamm commented 7 years ago

Ok, yes it will parse the nested IF blocks fine, but writes them back out wonky.

duncan-r commented 7 years ago

Ha! Good news almost :) Yes I remember those fun and games.

I just thought that there is one reason for changing the order of the parts and that is moving them in and out if logic blocks. This may be handled differently now?

I wonder if the API side could do similar to before...take the node to move and the node to put it before/after and just shift it there? Obviously the implementation of this is another matter! It seems to me that the biggest stumbling block is deciding how to reorder the children if the one you move? I.e. Where to attach them. This might depend in context? Do they go with it? Get attached to the next one up?

JamesRamm commented 7 years ago

Adding a part into a logic block would just be a case of locating the node with the logic block and calling append.

Moving a part out; find the node that contains the logic block, then call pop or remove on its children list...but letting people modify the children list like that is unsafe (as the parent will need to be correctly handled), so like append we might need pop, remove functions to handle this.