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

Merge simple and condition schedulers #60

Merged
merged 2 commits into from
May 5, 2021
Merged

Merge simple and condition schedulers #60

merged 2 commits into from
May 5, 2021

Conversation

kmantel
Copy link
Contributor

@kmantel kmantel commented May 5, 2021

The behavior of condition_scheduler with no conditions matches that in simple_scheduler. f060092 is optional but I think desired based on conversation with @jdcpni and @davidt0x

@pgleeson
Copy link
Member

pgleeson commented May 5, 2021

I think it makes sense to merge these, glad it was so straight forward.

One thing that would be nice though is if there was a wrapper class around the PNL specific classes/fields so that all the terms used (trial/timescale/sequence) are MDF's own and can be changed/updated easily on our side without impacting PNL. Also the "innards" of that wrapper class can eventually substituted with a MDF specific implentation (or a standalone schedluer) without too much additional changes...

@pgleeson
Copy link
Member

pgleeson commented May 5, 2021

Thinking a bit more about this I feel this file (modeci_mdf/scheduler.py) is the proper place for the "wrapper" class/fields (i.e. the MDFScheduler) which hides the specifics of PNI in an MDF specific API, and the EvaluableGraph could be extracted out & just use that API...

@kmantel kmantel merged commit f9ef86a into ModECI:main May 5, 2021
@kmantel kmantel mentioned this pull request May 5, 2021
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 this pull request may close these issues.

None yet

2 participants