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

Expand get_decision_problem_results with kwargs #1060

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

tengis-nrl
Copy link
Collaborator

@tengis-nrl tengis-nrl commented Feb 24, 2024

Tests passing
image

src/operation/decision_model.jl Outdated Show resolved Hide resolved
src/simulation/simulation_results.jl Outdated Show resolved Hide resolved
src/simulation/simulation_results.jl Outdated Show resolved Hide resolved
test/test_simulation_results.jl Outdated Show resolved Hide resolved
test/test_simulation_results.jl Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Feb 24, 2024

Performance Results

Version Precompile Time
Main 3.291137926
This Branch 3.255503522
Version Build Time
Main-Build Time Precompile 54.574988168
Main-Build Time Postcompile 2.714479927
This Branch-Build Time Precompile 52.324541887
This Branch-Build Time Postcompile 2.896360294

Copy link

codecov bot commented Feb 24, 2024

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 80.52%. Comparing base (12288cd) to head (8bd4923).
Report is 20 commits behind head on psy4.

❗ Current head 8bd4923 differs from pull request most recent head 2752563. Consider uploading reports for the commit 2752563 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             psy4    #1060      +/-   ##
==========================================
+ Coverage   80.48%   80.52%   +0.04%     
==========================================
  Files         115      115              
  Lines       12403    12411       +8     
==========================================
+ Hits         9982     9994      +12     
+ Misses       2421     2417       -4     
Flag Coverage Δ
unittests 80.52% <76.92%> (+0.04%) ⬆️

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

Files Coverage Δ
src/simulation/simulation_problem_results.jl 83.44% <0.00%> (+2.19%) ⬆️
src/simulation/simulation_results.jl 82.94% <83.33%> (-0.19%) ⬇️

... and 1 file with indirect coverage changes

src/simulation/simulation_results.jl Show resolved Hide resolved
src/simulation/simulation_results.jl Outdated Show resolved Hide resolved
)
@test !isnothing(PSI.get_system(results_ed))
@test PSY.get_units_base(get_system(results_ed)) == "DEVICE_BASE"

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also test that the errors work as expected when we can't find the system file and when we set populate_units without setting populate_system?

Copy link
Member

Choose a reason for hiding this comment

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

once this test is implemented we can merge

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @GabrielKS, could you confirm or deny the implementation of the test?

@tengis-nrl
Copy link
Collaborator Author

image The test for testing "file not found" error passes

@tengis-nrl tengis-nrl requested review from GabrielKS and removed request for GabrielKS March 1, 2024 00:05
@jd-lara jd-lara merged commit d530d20 into psy4 Mar 6, 2024
1 of 6 checks passed
@jd-lara jd-lara deleted the td/adding-kwargs-to-get_decision_problem_results branch March 6, 2024 15:15
daniel-thom added a commit that referenced this pull request Mar 6, 2024
daniel-thom added a commit that referenced this pull request Mar 6, 2024
jd-lara added a commit that referenced this pull request Mar 6, 2024
jd-lara pushed a commit that referenced this pull request Mar 27, 2024
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.

3 participants