-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix issue 181 #189
Fix issue 181 #189
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.
Looks good, nice work! I checked the model results and there seems to be no change at all between main
and this branch.
It would be nice to have docstrings in many of the new functions though (what does the function do? what are the inputs? what's the source of the equations?). But I know you can't write those. Perhaps keeping track of the functions that still need a docstring in an issue could be a good idea?
thanks a lot for reviewing and running the code. I agree that the code needs more documentation, I submitted issue #191 for this. |
❗ 🔴 Waiting for addressing issues mentioned here before the merge. |
Description
closes #181, the merge is pending addressing issues mentioned here. This branch re-produces the same results as those generated by the main branch, tested with a small dataset.
Checklist
Unreleased
.