search

LDAR-Sim / LDAR_Sim

MIT License
11 stars 22 forks source link

Enhc: Update Stacked "Costs" Comparison to Value Comparison for correctness #246

Closed ThomasGalesloot closed 4 months ago

ThomasGalesloot commented 4 months ago

Pull Request Key Information

Reason for change

The "stacked cost comparison" visualization is technically a value visualization, not a cost visualization, as cost is shown as negative.

What was changed

This change updates the name of the visualization and the y-axis label to reflect that the visualization is a value visualization.

Intended Purpose

Wording now updated to specify the visualization including program cost is value.

Level of version change required

Patch

Testing Completed

Manual Testing, see attached images of test run with simple test case 1:

image

Program Cost Value Comparison

All unit tests pass: unit_test_results.txt

Target Issue

Closes #230

Additional Information

N/A

ThomasGalesloot commented 4 months ago

Great job. Could you double check the param_default_const.py before you merge? I believe output defaults are in there as well... Need to also create an issue to re-organize the constants

None of the summary visualization names are in the output params in param default const - but they probably should be. The param default const really should be reorganized into groups to match output params anyways. Will add a ticket for that.