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

compute_output_current in system base #351

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

m-bossart
Copy link
Collaborator

To match postprocessing methods for other models, compute_output_current should always return currents in the system base for StandardLoad, PowerLoad, and ExponentialLoad . This change removes the dependency on the units setting of the system.

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #351 (c292d27) into main (deffaae) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
+ Coverage   87.30%   87.40%   +0.10%     
==========================================
  Files          65       65              
  Lines        8986     8995       +9     
==========================================
+ Hits         7845     7862      +17     
+ Misses       1141     1133       -8     
Flag Coverage Δ
unittests 87.40% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/post_processing/post_proc_loads.jl 97.19% <100.00%> (+14.54%) ⬆️

... and 1 file with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

Performance Results

Version Precompile Time
Main 5.115656155
This Branch 5.258909785
Version Execute Time
Main-Build ResidualModel 13.377871785
Main-Execute ResidualModel 34.080619047
Main-Build MassMatrixModel 1.137557307
Main-Execute MassMatrixModel 360.984364024
This Branch-Build ResidualModel 13.168182354
This Branch-Execute ResidualModel 35.892133313
This Branch-Build MassMatrixModel 1.131606052
This Branch-Execute MassMatrixModel 356.954510069

ResidualModel and MassMatrixModel performance results should be compared between versions and not between models due to the execution order of the tests

@m-bossart
Copy link
Collaborator Author

Doesn't hit the codecov target because the method for PowerLoad is not tested. Is PowerLoad still supported? Should I add a test for PowerLoad?

@jd-lara jd-lara requested review from jd-lara and removed request for rodrigomha August 29, 2023 20:31
@jd-lara jd-lara merged commit c67aa16 into main Aug 29, 2023
8 checks passed
@jd-lara jd-lara deleted the mb/output-current-loads branch August 29, 2023 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants