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

Enable overridable master algorithm selection for SSP #373

Merged
merged 12 commits into from
Sep 18, 2019

Conversation

markaren
Copy link
Contributor

@markaren markaren commented Sep 16, 2019

If accepted, this PR will replace #368, and will close #362 in the process.
It also replaces #370. It's easier to do both at once and will close #369

Instead of completely removing the custom annotation, it can function as a optional fallback similar to how defaultExperiment works in SSP and FMI.

The C++ API stays backwards compatible. The C API now requires 1 additional arguments for the cse_ssp_execution_create function, allowing the startTime defined within the ssp to be optinally used. Otherwise this will function as before.

A new function called cse_ssp_fixed_step_execution_create that takes 4 arguments is also introduced. This function allows the user to assign a custom stepSize value for the algoriithm. In the future this function should be replaced with a generic function taking a cse_algorithm pointer as argument instead. But that`s future work when cse ships with more than one master algorithm.

This PR should make everyone happy, happy, happy. Please have a look at the updated C API doc for the aforementioned function to see if it sounds ok though ..

It should also fix #338, as this PR makes the osp:FixedStepMaster element optional.

@ljamt
Copy link
Member

ljamt commented Sep 17, 2019

This is a better approach. Let's continue on this PR and close #368 and #370.

Can we increase test coverage of the introduced code?

@markaren
Copy link
Contributor Author

Can we increase test coverage of the introduced code?

Two new tests added.

Copy link
Member

@eidekrist eidekrist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@markaren markaren merged commit ce550a8 into master Sep 18, 2019
@markaren markaren deleted the ssp-overridable-master-algorithm branch September 18, 2019 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants