VeriFIT / mata

A fast and simple automata library
MIT License
19 stars 13 forks source link

`num_of_states()` is both in nfa and delta #340

Open jurajsic opened 12 months ago

jurajsic commented 12 months ago

Nfa's num_of_states() returns the maximum(max_initial, max_final, delta.num_of_states()). Delta's num_of_states returns the number of sources (is this correct? will it also count the targets?). We should probably rename delta's function.

Adda0 commented 12 months ago

Delta's num_of_states returns the number of sources (is this correct? will it also count the targets?).

No, it is not correct. In the newest iteration of delta design, Delta allocates space for both the sources and the targets. There cannot be a target for which there is no state post allocated. Therefore, Delta::num_of_states() returns the size of the vector of StatePost, which is precisely the largest source or target currently in Delta, or previously in Delta when defragmentation was not executed, + 1. Therefore, the naming is correct in my opinion. Is there anything you would like to discuss further regarding this issue? If not, feel free to close the issue.

jurajsic commented 12 months ago

It is a bit confusing to have two functions num_of_states which do not return the same thing. I would maybe rename delta's function to something else.

Adda0 commented 11 months ago

I do not understand. They return the same thing. Number of states in Nfa is defined as stated above and similarly, number of states in Delta is defined as I said above. Could you please explain what exactly you find different between these two methods?

I would have no problem with renaming Delta::num_of_states() to something else. But what would you like to call the function? It returns a number of states between state 0 and the largest (previously; as in, allocated) used state in transition relation.

jurajsic commented 11 months ago

In Nfa, it also adds the states from initial/final sets, in Delta it is only the states in the transition relation.

jurajsic commented 11 months ago

I am not sure how to rename it, maybe num_of_states is not such a bad name in delta either, it just might lead to confusion.

Adda0 commented 11 months ago

In Nfa, it also adds the states from initial/final sets, in Delta it is only the states in the transition relation.

This is the intended behaviour. I do not see a problem here, but I understand what you mean by causing confusion now. You either type nfa.delta.num_of_states() meaning you want a number of states in delta (but it might not be obvious if you skip the .delta. part), or you type nfa.num_of_states() meaning you want the number of states in the whole NFA (that is always obvious).

I personally consider this to be understandable and it does not confuse me. I cannot think of any other reasonable name, either. If we have an idea, I would be open to renaming the method in Delta.

jurajsic commented 11 months ago

It's just a bit weird to ask for number of states in delta, it could look like it is the same thing as for nfa, but yeah, let's just keep it, you can close this issue.

Adda0 commented 11 months ago

We will see whether someone will think of a better name.

Anyone else any opinions or ideas?

Adda0 commented 2 months ago

What about something like Nfa::num_of_states() and Nfa::Delta::num_of_transition_states() (as in, states used in transitions), to stress the difference? We already have Nfa::Delta::num_of_transitions(), so Nfa::Delta::num_of_transition_states() would go well together with it.

Adda0 commented 2 months ago

@kilohsakul Any opinions?