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

suspicous NEMA phase names #9971

Closed
namdre opened this issue Jan 18, 2022 · 7 comments
Closed

suspicous NEMA phase names #9971

namdre opened this issue Jan 18, 2022 · 7 comments

Comments

@namdre
Copy link
Contributor

namdre commented Jan 18, 2022

In the complex test results (i.e. complex/traci/trafficlight/NEMA/offset), the states change but the phase names do not:

state="grrrrrrGGrrr" name="4+4"/>
state="grrrrrrGGrrr" name="4+4"/>
state="grrrrrrGGrrr" name="4+4"/>
state="grrrrrrGGrrr" name="4+4"/>
state="grrrrrrGGrrr" name="4+4"/>
state="grrrrrrGGrrr" name="4+4"/>
state="GGGGGrrrrrrr" name="4+4"/>
state="GGGGGrrrrrrr" name="4+4"/>
state="GGGGGrrrrrrr" name="4+4"/>
state="GGGGGrrrrrrr" name="4+4"/>

Since the first traci action comes at step 30, this seems to be an issue with the basic operation (or maybe a faulty configuration?).

@qichaow @mschrader15 can you take a look?

@mschrader15
Copy link
Contributor

I will look into this

@mschrader15
Copy link
Contributor

mschrader15 commented Jan 18, 2022

@namdre It looks like that has been an issue since commit 77cdc5e24bd6bc15cee3db98f9e9fa134d7580f1 tests/complex/traci/trafficlight/NEMA/offset/tls_state.complex (here for reference)

I think it was an error in initialization that I thought I solved with Lines 607-609 in 1cc8ec9. I will do some more testing to confirm that I fixed this.

@mschrader15
Copy link
Contributor

@mschrader15
Copy link
Contributor

@namdre I am not seeing the error in tests run on HEAD. I believe that 1cc8ec9 resolved this.

@namdre
Copy link
Contributor Author

namdre commented Jan 18, 2022

You are right. Sorry for the bother )-:
I was looking at those tests to see why they produce different results after your commit but failed to see that it solved this very problem.

@namdre namdre closed this as completed Jan 18, 2022
@mschrader15
Copy link
Contributor

No problem!

For future reference, do you prefer PRs that address multiple issues or should I try and keep one PR per Issue? My mess of PRs yesterday was pretty confusing lol

@namdre
Copy link
Contributor Author

namdre commented Jan 18, 2022

One PR per issue would be preferable but it's not an iron rule. Sometimes one simple change fixes multiple bugs and it would be silly to try and separate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants