-
Notifications
You must be signed in to change notification settings - Fork 3
libcosimpy update - Ecco algorithm, conan update, type checking etc. #15
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
Conversation
- Code quality fix
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.
LGTM.
src/libcosimpy/CosimAlgorithm.py
Outdated
@dataclass | ||
class EccoParams(Structure): | ||
""" | ||
Ecco algorithm parameters. All parameters are in seconds. |
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.
All parameters are in seconds.
Are they? At least p_gain
, safety_factor
I'd expect to be unitless.
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.
True, will update it
Update to libcosimpy for supporting ecco algorithm.
Note, we can start integrating https://github.com/pybind/pybind11 gradually so that libcosimpy uses libcosim directly (not libcosimc). Though this requires (almost complete) rewrite of libcosimpy.