Validate allocation input

[Jingru923]

Fixes #710

In the validation.jl I created three functions to address validation for the positive allocation id, non-negative use demand, and for complete subnetwork. Unit tests are made for the three functions. The second task in issue #710 was skipped because it depends on main_network_allocation_main branch which is not yet finished.

[SouthEndMusic]

I added some suggestions. I was also thinking about how to organize the test, because triggering different validation errors with the same model might be complicated due to the structure of the initialization code. Here are my thoughts per task of the issue:

• For this one it is probably best to create an invalid test model in Ribasim Python as you already started making.
• For this one I was a bit naughty, as I wrote the mentioned source validation function without testing it. Maybe you can skip this one for now, as I changed this validation function in the branch I am working on.
• For this one it might be educational to create an instance of a User object within the test, just as is done for instance here for Basin. Also, you said you created an if statement in the inner constructor, but function User is not an inner constructor. Doing validation in an inner constructor looks something like this:

struct Circle
   r::Float64
   function Circle(r::Float64)
      if r < 0
         error("Circle radius must be non-negative, got $r.")
      end
      return new(r)
   end
end

We the same in our own code for instance here.

• The last one is probably also best done with a model made in Ribasim python. Creating a subnetwork is quite easy: you only have to add a column to the Node table called allocation_network_id, see also here in the docs.

Also a small nitpick:

• In the description of the PR you put the number of this PR instead of the number of the issue (710).

[SouthEndMusic]

This might work, but its quite hard to read. I propose you write a nested loop:

for (user_node_id, demand_user) in zip(node_ids, interpolations)
   for (itp, priority) in zip(demand_user, priorities)
     ...
   end
end

This way you have the right information available to write a helpful error message, and you can use the same structure as above to check all data before crashing the program if a problem was found.

[SouthEndMusic]

nitpick:

Suggested change

	error("Demand should be a non-negative number")
	error("Demand should be a non-negative float")

[SouthEndMusic]

I also saw that this docstring is not correct, so you can fix my mistake (:

Ribasim/core/src/create.jl

Line 644 in 306d29a

# The highest priority number given, which corresponds to the least important demands

It should be something like # All provided priorities.

On a related note, we use a VSCode extension called Git Blame, which tells you at the bottom of your screen who wrote a line of code you don't like if you put your cursor on it 😄

[sonarqubecloud]

Quality Gate failed

Failed conditions

66.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

[SouthEndMusic]

Regarding this test: It is good practice to create your own test model to do this, but I thought of an easier way: load the parameters of an existing model, say

toml_path = normpath(@__DIR__, "../../generated_testmodels/allocation_example/ribasim.toml")
p = Ribasim.Model(toml_path).integrator.p

remove an edge to make it invalid and run your validation test on that.

[Jingru923]

This morning I tried to load an existing model and change one of the allocation id to negative number. It gave error saying the immutable struct of type cannot be changed. I remembered you mentioned this yesterday, it probably can't be changed or maybe I didn't do it the correct way. Anyhow, in my new code I created a graph that has the required feature and used it as the test model.

[Hofer-Julian]

Looks to me like merging main into your branch went wrong.
This needs to be fixed up before we can review this :)

[SouthEndMusic]

Looks good, I added a few small comments.