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

Add docstrings to RustSimDriveParams #141

Merged
merged 12 commits into from
Jun 28, 2024
Merged

Conversation

michael-okeefe
Copy link
Collaborator

@michael-okeefe michael-okeefe commented Jun 27, 2024

Fixes: https://github.nrel.gov/MBAP/fastsim/issues/348

The docstrings were added by referencing other parts of the code. Some were slightly adjusted to enhance clarity where possible.

NOTE: this also incorporates a change that seems to have fixed the unit tests not building in Github actions:

The root cause appears to be a breaking change in numpy 1.24 and later that causes an error in another dependency (pymoo?). I limited the max version of numpy to be <1.24 to fix. A more comprehensive fix might involve updating all the deps.

Michael O'Keefe added 5 commits January 9, 2024 07:30
@michael-okeefe michael-okeefe added the documentation Improvements or additions to documentation label Jun 27, 2024
@michael-okeefe michael-okeefe self-assigned this Jun 27, 2024
Based on:
actions/runner#2468

Test failures appear to occur due to an out-of-memory error. This attempts to increase the swap-space size on Ubuntu.
The change to increase swap space didn't seem to help so reverting.
@michael-okeefe
Copy link
Collaborator Author

NOTE: I noticed the unit tests don't seem to be passing via Github Actions. I tried an experiment based on this thread to increase the swap space thinking it might have been a memory issue but that didn't work:

actions/runner#2468

Not sure at this point what is wrong with the automated tests although I wonder if it could be the "vehicle import" tooling since that requires network access? Just a wild guess at this point...

Based on this thread, this could be a cause of the
unit tests failing? Trying...
@michael-okeefe
Copy link
Collaborator Author

OK, not sure if this definitively fixed the unit tests but it appears to be a breaking change in numpy 1.24 and later that causes this. This shows up in one of the dependencies. I limited the max version of numpy to be <1.24 to fix but there's probably a better, more comprehensive fix involving updating all the deps.

pub favor_grade_accuracy: bool,
pub missed_trace_correction: bool, // if true, missed trace correction is active, default = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jakeholden , check out these doc strings

@calbaker
Copy link
Collaborator

Also addresses #9

@jakeholden jakeholden self-requested a review June 28, 2024 19:07
Copy link
Collaborator

@jakeholden jakeholden left a comment

Choose a reason for hiding this comment

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

These new docs strings look excellent!!

@jakeholden jakeholden merged commit c747884 into fastsim-2 Jun 28, 2024
@jakeholden jakeholden deleted the fix/issue-348-doc-strings branch June 28, 2024 19:28
@calbaker
Copy link
Collaborator

calbaker commented Jul 1, 2024

@jakeholden , @michael-okeefe should get most of the credit for this, FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants