-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adding EMCF solver #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I've added a few non-blocking issues, which we can tackle later.
I will leave this branch till @gmgunter gets a chance to take a better look. No rush. The feature branch should be fully functional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience! I'm starting to catch back up now
I think the comments here have come in a little too late to address them satisfactorily and not impact the two other PRs that are queued up. I can included them in the last of the PRs that are stringed together. Does that work? |
Yeah, that's totally fine with me. I'll continue adding comments as I read through the changes, but they can be addressed in subsequent PRs. I'll approve each of the backlog PRs once I'm finished reviewing it, even if there are remaining comments, until I'm caught back up with you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM, let's go ahead and merge this one. I appreciate it if you could address the comments below in a subsequent PR (or respond to them if they're off-base).
tests/workflows/test_emcf.py