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.

Sign up for GitHub

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

Jump to bottom

feat: add ProfitOrLossReporter to compute profit/loss of energy flows #808

Merged
merged 17 commits into from Aug 22, 2023
Merged

feat: add ProfitOrLossReporter to compute profit/loss of energy flows #808

merged 17 commits into from Aug 22, 2023

Conversation

victorgarcia98
Copy link
Contributor

@victorgarcia98 victorgarcia98 commented Aug 10, 2023 •
edited

Description

This PR adds a new type of reporter: ProfitReporter. This reporter takes a production and consumption price and computes the cashflow given an energy or power sensor, applying the conversions required.

How to test

You can find the tests in the following link:

fa7b2de#diff-5808e1304f6229bb4daae51719b6d9f17a8cd6469e4c3cf963366858c954f8b5

Further Improvements

This is reporter is going to show up in the coming tutorial for reporting.

TODO

  • Test this reporter in a DST transition.
  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

@victorgarcia98
add CostReporter with tests
fa7b2de 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 self-assigned this Aug 10, 2023
Flix6x
Flix6x requested changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

As with every new reporter, I'm looking forward to using this in production. I reviewed everything except the tests. My main issues are with naming of the output sensor (which is probably only a matter of internal aliases) and unit/sign validation.

flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/cost.py Outdated Show resolved Hide resolved
@Flix6x Flix6x added this to the 0.16.0 milestone Aug 11, 2023
@victorgarcia98
rename CostReporter to profit reporter
3fe8a35 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
improve validation
82202f2 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
Merge remote-tracking branch 'origin/main' into feature/reporting/cos…
a0af4ba 
…t-reporter
@victorgarcia98
Merge remote-tracking branch 'origin/feature/reporting/cost-reporter'…
a6d77f7 
… into feature/reporting/cost-reporter
@victorgarcia98 victorgarcia98 changed the title add CostReporter to compute cashflows due to energy flows add ProfitReporter to compute cashflows due to energy flows Aug 14, 2023
@victorgarcia98 victorgarcia98 changed the title add ProfitReporter to compute cashflows due to energy flows add ProfitReporter to compute profit of energy flows Aug 14, 2023
Flix6x
Flix6x requested changes Aug 18, 2023
View reviewed changes
flexmeasures/data/models/reporting/profit.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/profit.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/profit.py Outdated Show resolved Hide resolved
@victorgarcia98
Merge branch 'main' into feature/reporting/cost-reporter
a643a6a
@victorgarcia98
fix comments
4b4c4c5 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
Add is_profit to ProfitOrLossReporter
2c1f833 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
victorgarcia98 and others added 2 commits August 21, 2023 09:45
@victorgarcia98 @Flix6x
Call a Reporter saved in the DB via CLI (#818)
a763155 
* add DataSource field to flexmeasures add report

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add new command to create basic sources for the available reporters.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* Switch off HiGHS logs for `LOGGING_LEVEL=INFO` (#824)

* add log toggling for HiGHS

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* rename command + add changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* use for loop in flexmeasures add sources

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: typo

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: make 'kind' choice singular, so it matches the key in data_generators

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: add inline todo

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: get db from where we normally get it

Signed-off-by: F.N. Claessen <felix@seita.nl>

* style: black

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Co-authored-by: F.N. Claessen <felix@seita.nl>
@victorgarcia98
Make source optional in the input field of the AggregatorReporter (#…
47cce5f 
…819)

* add DataSource field to flexmeasures add report

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix: use source and make it an optional field

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* fix tests

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add test + fix case of multiple sources

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add new command to create basic sources for the available reporters.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* set sensor in ouput bdf

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* set resolution

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* update docstring

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* add changelog entry

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* rename command + add changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* allow passing multiple sources

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* extend changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* clarify docstring

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* check sources

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* use for loop in flexmeasures add sources

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor <victor@seita.nl>
Flix6x
Flix6x requested changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Almost there. Very nice to see the boolean setting working so elegantly (without needing to have additional Reporter classes and duplicate code).

flexmeasures/data/models/reporting/profit.py Outdated Show resolved Hide resolved
flexmeasures/data/models/reporting/profit.py Outdated Show resolved Hide resolved
victorgarcia98 and others added 4 commits August 22, 2023 14:43
@victorgarcia98
rename is_profit to profit_is_positive
929e945 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
fix: highs string is in solver_name (#826)
bc291b1 
* fix:  `highs` string is in `solve_name` using `in`

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

* chore: update changelog

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>

---------

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@Flix6x @victorgarcia98
Fix: public sensors to show (#830)
1564eda 
Fix two bugs dealing with public assets.


* fix: public sensors have None owner

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

* fix: searching for annotations on public assets

Signed-off-by: F.N. Claessen <felix@seita.nl>

* refactor: more explicit if-statement

Signed-off-by: F.N. Claessen <felix@seita.nl>

* docs: add second fix to changelog

Signed-off-by: F.N. Claessen <felix@seita.nl>

---------

Signed-off-by: F.N. Claessen <felix@seita.nl>
@victorgarcia98
Merge branch 'main' into feature/reporting/cost-reporter
78f79e6 
Signed-off-by: Victor <victor@seita.nl>
@Flix6x Flix6x changed the title add ProfitReporter to compute profit of energy flows feature:add ProfitOrLossReporter to compute profit/loss of energy flows Aug 22, 2023
@Flix6x Flix6x changed the title feature:add ProfitOrLossReporter to compute profit/loss of energy flows feat: add ProfitOrLossReporter to compute profit/loss of energy flows Aug 22, 2023
Flix6x
Flix6x approved these changes Aug 22, 2023
View reviewed changes
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

🎉 Don't forget the changelog entry.

@victorgarcia98
add changelog entry
a70b2e2 
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98
Merge remote-tracking branch 'origin/feature/reporting/cost-reporter'…
1603e6a 
… into feature/reporting/cost-reporter
@victorgarcia98 victorgarcia98 merged commit a21059a into main Aug 22, 2023
4 of 5 checks passed
@victorgarcia98 victorgarcia98 deleted the feature/reporting/cost-reporter branch August 22, 2023 15:27
Flix6x
Flix6x reviewed Aug 22, 2023
View reviewed changes
documentation/changelog.rst
@@ -6,6 +6,12 @@ FlexMeasures Changelog
v0.15.1 | August XX, 2023
============================

New features
Copy link
Contributor

Choose a reason for hiding this comment

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

(Aye, you put this under 0.15.1 instead of 0.16.)

@victorgarcia98 victorgarcia98 mentioned this pull request Sep 12, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flix6x Flix6x Flix6x approved these changes

Assignees

@victorgarcia98 victorgarcia98

Labels
Reporting
Projects
None yet
Milestone
0.16.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@victorgarcia98 @Flix6x