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 proforma to API v3 #553

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

Add proforma to API v3 #553

wants to merge 6 commits into from

Conversation

rathod-b
Copy link
Collaborator

Please check if the PR fulfills these requirements

  • CHANGELOG.md is updated
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

(Bug fix, feature, docs update, ...)

What is the current behavior?

(You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

(What changes might users need to make in their application due to this PR?)

Other information:

Standalone python testing to create proforma and do negative testing
"\n",
"inandout_sheet.write('B1', '', d['header_format_right'])\n",
"\n",
"inandout_sheet.write('B2', '', d['subheader_format_right'])\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could probably for-loop this section?

@adfarth
Copy link
Collaborator

adfarth commented Dec 22, 2023

Summary of some initial testing:

  • 3/3 testing results jsons (attached below) produced errors in the code
    diesel_prime.json
    pv_bess_primegen.json
    web_tool_sample_site.json
  • It doesn't look like there's any accounting for Third-Party ownership? Is that just TBD?
  • In the old proforma (in most but not all places) fractions/percentages are reported as the whole number value (e.g. 8.3 instead of 0.083), which matters mostly because some of these fractions are then used in equations (e.g., the IRR equation). Since your values are all fractions, the equations are wrong. I think outputting them as fractions, modifying the formatting where needed, and changing the equations to not divide by 100 is probably the best path forward.
  • It does seem like the old code formatted things as "Numbers" or "Percentages" whereas this one all appears to have the "General" number format, or "Scientific" in some cases.

@rathod-b
Copy link
Collaborator Author

rathod-b commented May 9, 2024

@adfarth @Bill-Becker the branch now has proforma code integrated into it (i need to clean up the code a bit more). You can run a scenario, then supply its run_uuid to the new v3 proforma endpoint (GET), which returns an excel file of proforma in response.

@Bill-Becker
Copy link
Collaborator

Summary of some initial testing:

  • 3/3 testing results jsons (attached below) produced errors in the code
    diesel_prime.json
    pv_bess_primegen.json
    web_tool_sample_site.json
  • It doesn't look like there's any accounting for Third-Party ownership? Is that just TBD?
  • In the old proforma (in most but not all places) fractions/percentages are reported as the whole number value (e.g. 8.3 instead of 0.083), which matters mostly because some of these fractions are then used in equations (e.g., the IRR equation). Since your values are all fractions, the equations are wrong. I think outputting them as fractions, modifying the formatting where needed, and changing the equations to not divide by 100 is probably the best path forward.
  • It does seem like the old code formatted things as "Numbers" or "Percentages" whereas this one all appears to have the "General" number format, or "Scientific" in some cases.

@rathod-b looks like the fractions vs percent is still an issue, where the values are fractions but they are being handled as percents in the equations which divide them by 100.

Also, and I mostly expected this, but I ran a CHP case, and the boiler and CHP fuel costs are not being handled (zeros). I think this is an update that needs to come from the REopt.jl proforma script(s).

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.

None yet

3 participants