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

Restarting the same stream #98

Open
mr-j0nes opened this issue May 9, 2020 · 6 comments · May be fixed by #126
Open

Restarting the same stream #98

mr-j0nes opened this issue May 9, 2020 · 6 comments · May be fixed by #126

Comments

@mr-j0nes
Copy link
Collaborator

mr-j0nes commented May 9, 2020

Hi Jonathan,

very quick question, is there any particular reason why a stream cannot be restarted without initializing the map ? E.g.:

    raft::map m;
    m += a >> b >> c;
    m.exe();
    m.exe();  // <-- throws exception

The exception thrown is:

terminate called after throwing an instance of 'PortExceptionBase<1>'
  what():  Source port "out" already initialized!

Thanks a lot,
cheers.

@jonathan-beard
Copy link
Member

jonathan-beard commented May 10, 2020

Good point...likely we should allow that.

Short answer

I wanted to throw an exception if people try to attach to the same port twice and also have an exception for when the runtime itself errs and tries to double allocate things (as a sanity check). It doesn't have to throw an exception at each exe(), it could be smart enough not to throw an exception from there (the exe()) but only from the >> operators.

Longer answer

The graph is built lazily, and by that I mean that nothing is really evaluated aside from connectivity and type until you call exe. So what the exception is saying is that the runtime is trying to allocate another FIFO for something that already has one. Again, I think you're right @mr-j0nes, we should allow the exe() to be called again. Shouldn't be too hard to fix up.

No problem! Keep the questions coming. I'll see if I can get to some of the rest of them.

@mr-j0nes
Copy link
Collaborator Author

Exactly. Now I am trying to implement a recovering logic which requires the stream to be restarted. It does not feel right to have to re-link all kernels while they are already linked and nothing has to change in that area.

Good that you also agree, it seems to me that all checks and evaluations should be done at the kernel linking stage rather than in the exe().

Perfect, then I wait until you have a bit more of time.

Cheers..

@jonathan-beard
Copy link
Member

HI @mr-j0nes, I'm working on a fix, right now I'm gonna have a check in the exe for linking just as before, but there will be a bool to ensure it's not done twice. Seems like a good interim fix. Almost passing test cases, but, it seems there's a stack vs. heap alloc issue.

@jonathan-beard jonathan-beard linked a pull request Jul 21, 2020 that will close this issue
16 tasks
@mr-j0nes
Copy link
Collaborator Author

Hi Jonathan, hope you are doing fine.
I just remember this change and wanted to know if there is something there that I can test ? Is the feature already implemented ?

@jonathan-beard
Copy link
Member

It's in progress. There's shutdown condition that I need to deal with. More specifically, There needs to be a cleaner way of doing "barriers" to wait on the specific kernel that is needed within the main thread of execution.

@mr-j0nes
Copy link
Collaborator Author

I know there is a branch with work on this, but I have work around'ed this by just creating all kernels again in the second run:

void run()
{
  A a; B b; C c;
  raft::map m;
  m += a >> b >> c;
  m.exe();
}

in main()
{
  run();
  run(); // <-- second run successfully
}

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

Successfully merging a pull request may close this issue.

2 participants