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

Support latest graph_scheduler #496

Closed
pgleeson opened this issue Feb 1, 2024 · 5 comments
Closed

Support latest graph_scheduler #496

pgleeson opened this issue Feb 1, 2024 · 5 comments

Comments

@pgleeson
Copy link
Member

pgleeson commented Feb 1, 2024

Current version only works with graph_scheduler<1.2.0, e.g. see failing tests at:

https://github.com/ModECI/MDF/actions/runs/7739807955

Any idea what's changed in the latest graph_scheduler @kmantel?

@kmantel
Copy link
Contributor

kmantel commented Feb 1, 2024

There's a new check for invalid termination condition input, and the two failing tests look to be hitting that. It looks like they don't specify a time scale for the termination condition:

"termination": {
"check_term_true": {
"type": "JustRan",
"kwargs": {
"dependencies": [
"check_termination"
]
}
}
}

A correct example:

"termination": {
"environment_sequence": {
"type": "Never",
"args": {}
},
"environment_state_update": {
"type": "AllHaveRun",
"args": {
"dependencies": []
}
}
}
},

In prior graph-scheduler versions, that condition would have been silently ignored.

This line looks like the culprit:

termination={"check_term_true": cond_term},

@kmantel
Copy link
Contributor

kmantel commented Feb 1, 2024

I'm not sure the exact intent of the model, but my guess would be that

 termination={"environment_state_update": cond_term}, 

would be the replacement to make.

Maybe @davidt0x has input

@pgleeson
Copy link
Member Author

@davidt0x Did you suggest in our last call that your latest PR solved this issue?

@davidt0x
Copy link
Contributor

I did suggest that but I forgot to include the fix, will generate another PR, sorry for the confusion.

@davidt0x
Copy link
Contributor

This issue is fixed with PR #524

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

No branches or pull requests

3 participants