Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

num_of_states() is both in nfa and delta #340

Open
jurajsic opened this issue Sep 22, 2023 · 10 comments
Open

num_of_states() is both in nfa and delta #340

jurajsic opened this issue Sep 22, 2023 · 10 comments
Labels
For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata

Comments

@jurajsic
Copy link
Member

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.

@jurajsic jurajsic added For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata labels Sep 22, 2023
@Adda0
Copy link
Collaborator

Adda0 commented Sep 23, 2023

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
Copy link
Member Author

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
Copy link
Collaborator

Adda0 commented Sep 25, 2023

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
Copy link
Member Author

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

@jurajsic
Copy link
Member Author

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
Copy link
Collaborator

Adda0 commented Sep 25, 2023

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
Copy link
Member Author

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
Copy link
Collaborator

Adda0 commented Sep 25, 2023

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

Anyone else any opinions or ideas?

@Adda0
Copy link
Collaborator

Adda0 commented Jun 25, 2024

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
Copy link
Collaborator

Adda0 commented Jun 25, 2024

@kilohsakul Any opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For:library The issue is related to library (c++ implementation) Module:nfa The issue is related to Nondeterministic Finite Automata
Projects
None yet
Development

No branches or pull requests

2 participants