Update the mvs parameters docs file by mahendrark · Pull Request #479 · rl-institut/multi-vector-simulator

mahendrark · 2020-07-27T15:20:39Z

Addressed part of #269

Changes proposed in this pull request:

Update the MVS parameters page on readthedocs

The following steps were realized, as well (if applies):

Use in-line comments to explain your code
Write docstrings to your code
For new functionalities: Explain in readthedocs
Write test(s) for your new patch of code
Update the CHANGELOG.md
Apply black (black . --exclude docs/)
Check if benchmark tests pass locally (EXECUTE_TESTS_ON=master pytest)

Please mark above checkboxes as following:

Open
Done

❌ Check not applicable to this PR

_{For more information on how to contribute check the CONTRIBUTING.md.}

smartie2076 · 2020-07-27T15:21:32Z

Proposal: Index of parameters (alphabetical) instead of list for each energyAsset type.

Parameter

unit: kW
type: str
example: "I_am_a_string"
restrictions: None
default: None
jdsaklfjdgepuewMCLKDS98E4JFEW

smartie2076 · 2020-07-29T07:51:45Z

@mahendrark I converted this to draft, as it is not ready to be merged.

mahendrark · 2020-08-05T15:51:27Z

@smartie2076

can you please check the parameters docs and let me know if this is how you wanted the parameters to appear ?

smartie2076

You can see the compiled rst file here (updates after about 5 Minutes)

https://mvs-eland.readthedocs.io/en/improve-docs/MVS_parameters.html

Formatting stuff

docs/MVS_parameters.rst

Co-authored-by: smartie2076 <[email protected]>

mahendrark · 2020-08-10T12:50:50Z

@smartie2076

Can you tell me how to address the Common Parameters in the CSV/JSON files and in energyConversion.csv ?

Should I move them all to their respective columns or keep them as it is now?

smartie2076 · 2020-08-11T08:21:29Z

Can you tell me how to address the Common Parameters in the CSV/JSON files and in energyConversion.csv ?

Should I move them all to their respective calendars or keep them as it is now?

Not sure what you mean with "calendars", but I was thinking of having an alphabetically sorted list of parameters in the file now (aka a parameter index). Another file (or in the same file in the head section) we then can simply enumerate the all inputs for each parameter group, and we add links to the respective entries in the parameter index. That way, we do not have to define a parameter multiple times.

smartie2076 · 2020-09-30T08:55:41Z

Hi @mahendrark! Can you tell me how far you got with this PR? We are aiming to close all PR asap.

Bachibouzouk · 2020-10-01T08:17:30Z

@mahendrark do you compile locally with html make and pay attention to the warnings?

docs/MVS_parameters.rst

Bachibouzouk

"Parameters and Definitions in CSVs/JSON" and "Parameters in each CSV file" seems to be two different sections according to the navigation bar (see picture)

However they appear on the same page. This might be due to the incorrect use of *** for title of subsections (instead of ---)

The number of -/*/= below a subsection/chapter/section's title should always match the number of characters of the title, this is not the case

I think we could separate the different definitions using ---- (4 times exactly and line breaks before and after), this will render as an horizontal line :)

Btw, this PR claims to fix #269, but it does not address any of the two points described in the description of #269, I would remove the mention "fix" to replace it with "addresses part of" so that the issue is not closed when we merge this PR :)

Bachibouzouk · 2020-10-01T08:51:26Z

Looking at the structure of the sections in readthedocs, I think there might be improvments:

"Parameters and Definitions in CSVs/JSON"
"Parameters in each CSV file"
"Outputs of the MVS simulation"
Belong to "Simulating with the MVS". I would merge "Parameters and Definitions in CSVs/JSON" and "Parameters in each CSV file" (as they are already in one file and it seems redundant section headings) and place it as a subsection of "Input files" (at the same heading level that "Csv files: csv_elements folder" and "Json file: mvs_config.json").

I would then link the csv names under "Csv files: csv_elements folder" to the "Parameters in each CSV file" section and keep the same order of csv file names.

smartie2076

You need to add a changelog entry.

Just a few comments, I did not check all definitions.

You can also check the formatting here: https://mvs-eland.readthedocs.io/en/improve-docs/MVS_parameters.html#

docs/MVS_parameters.rst

smartie2076 · 2020-10-01T09:20:36Z

  I would merge "Parameters and Definitions in CSVs/JSON" and "Parameters in each CSV file" (as they are already in one file and it seems redundant section headings)

+1

and place it as a subsection of "Input files" (at the same heading level that "Csv files: csv_elements folder" and "Json file: mvs_config.json").

+1 Eventhough the file will get really, really long. Will need to work out the correct section heads then. Maybe we also need to merge other currently seperate .rst files.

I would then link the csv names under "Csv files: csv_elements folder" to the "Parameters in each CSV file" section and keep the same order of csv file names.

I am not quite sure what you mean with that.

Co-authored-by: smartie2076 <[email protected]>

Bachibouzouk · 2020-10-01T09:27:54Z

and place it as a subsection of "Input files" (at the same heading level that "Csv files: csv_elements folder" and "Json file: mvs_config.json").

+1 Eventhough the file will get really, really long. Will need to work out the correct section heads then. Maybe we also need to merge other currently seperate .rst files.

We can use .. include:: <path to rst file> within a rst file to separate into more files. The section cut does not depend on splitting the content into several files (the compiler sees the whole content as one huge file) but on the section headings (we need to always know where we stand in the tableofcontent hierarchy if we use several files). For the documentation I think it is ok to have long files.

Bachibouzouk · 2020-10-01T09:31:03Z

I would then link the csv names under "Csv files: csv_elements folder" to the "Parameters in each CSV file" section and keep the same order of csv file names.

I am not quite sure what you mean with that.

Under the section heading "Simulating with the MVS/Input files/Csv files: csv_elements folder" we list the csv files in csv_element folder. We also do so in "Parameters in each CSV file", but not with the same order. I would go for alphabetical order as it is displayed in the input_template/csv_elements folder

mahendrark · 2020-10-01T09:55:56Z

@mahendrark do you compile locally with html make and pay attention to the warnings?

Yes, there are 18 warnings that not related to the code in the file that I am working on.

For example:

No module named 'mvs_eland.utils'
WARNING: autodoc: failed to import module 'E3_indicator_calculation' from module 'mvs_eland'; the following exception was raised:
No module named 'mvs_eland.utils'
WARNING: autodoc: failed to import module 'E4_verification_of_constraints' from module 'mvs_eland'; the following exception was raised:
No module named 'mvs_eland.utils'
WARNING: autodoc: failed to import module 'F0_output' from module 'mvs_eland'; the following exception was raised:
No module named 'mvs_eland.utils'
WARNING: autodoc: failed to import module 'F1_plotting' from module 'mvs_eland'; the following exception was raised:
No module named 'mvs_eland.utils'

@smartie2076

Co-authored-by: smartie2076 <[email protected]>

mahendrark · 2020-10-01T11:06:57Z

+1 Eventhough the file will get really, really long. Will need to work out the correct section heads then. Maybe we also need to merge other currently seperate .rst files.

I think this should be done in another PR since this one only addresses the parameters of MVS and not broader documentation.

Corrections in the restrictions

smartie2076 · 2020-10-01T12:33:54Z

@Bachibouzouk if you think the file is correct from the sphinx viewpoint, I would say we merge it and I fix the definitions if necessary in a new PR. Can you check the latest commits?

Bachibouzouk · 2020-10-01T16:08:09Z

docs/MVS_parameters.rst

+:Type: Numeric
+:Unit: currency
+:Example: 10000
+:Restrictions: Positive real numbers


Suggested change

:Restrictions: Positive real numbers

:Restrictions: Positive real number

Bachibouzouk · 2020-10-01T16:15:08Z

docs/MVS_parameters.rst

-**cost_om**: Specific annual OPEX of the asset (€/kW/year)
+:Definition: Stands for Demand Side Management. Currently, not implemented.
+:Type: str
+:Unit: Boolean value


@smartie2076
I find the fields describing a parameter would require a definition and a list of possible values as well. It is hard to understand what is a :Unit: in this example:
What is a Boolean value as a :Unit:. Shouln't :Type: be boolean and :Unit: be None?

Bachibouzouk · 2020-10-01T16:17:15Z

docs/MVS_parameters.rst

-**energyVector**: Energy commodity. E.g.: Electricity, heat, bio-gas, etc. 
+:Definition: Ratio of energy output/energy input. The battery efficiency is the ratio of the energy taken out from the battery, to the energy put in the battery. It means that it is not possible to retrieve as much energy as provided to the battery due to the discharge losses. The efficiency of the "input power" and "ouput power" columns should be set equal to the charge and dischage efficiencies respectively, while the "storage capacity" efficiency should be equal to the storage self-discharge/decay, which is usually in the range of 0 to 0.05.
+:Type: Numeric
+:Unit: Factor


Same remark here, how Factor is a unit comparable to kW ? Shouldn't the :Type: be factor?

Bachibouzouk · 2020-10-01T16:18:01Z

docs/MVS_parameters.rst

+:Type: Numeric
+:Unit: Days
+:Example: 365
+:Restrictions: None


Suggested change

:Restrictions: None

:Restrictions: Positive integer number

Bachibouzouk · 2020-10-01T16:19:14Z

docs/MVS_parameters.rst

+:Type: Numeric
+:Unit: Year
+:Example: 30
+:Restrictions: None


Suggested change

:Restrictions: None

:Restrictions: Positive integer number

Bachibouzouk

@mahendrark, @smartie2076 :
This PR is about inserting the references, the definition and correct units linked each parameter should be reviewed into a separate PR (created issue #586 for this), we can ignore my suggestion and merge

Update the mvs parameters docs file

da307a1

mahendrark self-assigned this Jul 28, 2020

mahendrark added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 28, 2020

smartie2076 marked this pull request as draft July 29, 2020 07:51

This was referenced Aug 3, 2020

Structure for input validity definitions #488

Closed

Map default types and values for parameters #441

Closed

Improve the description of project info parameter in readthedocs

67d7f0e

smartie2076 reviewed Aug 6, 2020

View reviewed changes

docs/MVS_parameters.rst Outdated Show resolved Hide resolved

docs/MVS_parameters.rst Outdated Show resolved Hide resolved

Create level 3 headline

d7ee3a2

Co-authored-by: smartie2076 <[email protected]>

Improve the readthedocs for MVS parameters

3daffaf

mahendrark added 3 commits September 24, 2020 17:44

Update and sort the parameters list horizontally

9a11b29

Updates to the parameter definitions in the docs file

a6e3e06

Update the parameter list with crossreferencing labels

8a18afa

mahendrark added 2 commits September 30, 2020 12:18

Update the CSV files list with the relevant parameters crossreferenced

8b2099a

Update parameter definitions and CSV lists, and improve formatting

6a2f104

mahendrark requested a review from smartie2076 September 30, 2020 12:27

mahendrark marked this pull request as ready for review September 30, 2020 12:28

mahendrark added 3 commits September 30, 2020 14:29

Merge branch 'dev' into improve-docs

1ade08a

Fix formatting of the headings for the csv files

7a7f87d

Merge remote-tracking branch 'origin/improve-docs' into improve-docs

701288d

Bachibouzouk reviewed Oct 1, 2020

View reviewed changes

docs/MVS_parameters.rst Outdated Show resolved Hide resolved

Bachibouzouk reviewed Oct 1, 2020

View reviewed changes

Bachibouzouk mentioned this pull request Oct 1, 2020

Readthedocs structure needs improvements #582

Closed

smartie2076 reviewed Oct 1, 2020

View reviewed changes

Remove the redundant sentence in the example of currency parameter

df6156e

Co-authored-by: smartie2076 <[email protected]>

Fix the heading levels such that the file is not split into two parts

0ebc345

mahendrark and others added 3 commits October 1, 2020 12:18

Remove the extra marker extra characters under headings

f241689

Insert horizontal lines between each successive parameter definition

5a25e2d

Update definition of parameter c-rate

019be08

Co-authored-by: smartie2076 <[email protected]>

mahendrark added 3 commits October 1, 2020 13:09

Modifications in defs of parameters age_installed & development_costs

b0ccbc5

Corrections in the restrictions

Merge remote-tracking branch 'origin/improve-docs' into improve-docs

716c59c

Update the CHANGELOG

45b4ccd

mahendrark requested a review from smartie2076 October 1, 2020 11:13

Merge branch 'dev' into improve-docs

39aee0a

Bachibouzouk reviewed Oct 1, 2020

View reviewed changes

Bachibouzouk approved these changes Oct 1, 2020

View reviewed changes

Bachibouzouk merged commit 3f4835f into dev Oct 1, 2020

Bachibouzouk deleted the improve-docs branch October 1, 2020 16:27

Bachibouzouk mentioned this pull request Oct 5, 2020

Release/v0.5.0 #589

Merged

	:Restrictions: Positive real numbers
	:Restrictions: Positive real number

Conversation

mahendrark commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smartie2076 commented Jul 27, 2020

Parameter

Uh oh!

smartie2076 commented Jul 29, 2020

Uh oh!

mahendrark commented Aug 5, 2020

Uh oh!

smartie2076 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahendrark commented Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smartie2076 commented Aug 11, 2020

Uh oh!

smartie2076 commented Sep 30, 2020

Uh oh!

Bachibouzouk commented Oct 1, 2020

Uh oh!

Uh oh!

Bachibouzouk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk commented Oct 1, 2020

Uh oh!

smartie2076 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smartie2076 commented Oct 1, 2020

Uh oh!

Bachibouzouk commented Oct 1, 2020

Uh oh!

Bachibouzouk commented Oct 1, 2020

Uh oh!

mahendrark commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mahendrark commented Oct 1, 2020

Uh oh!

smartie2076 commented Oct 1, 2020

Uh oh!

Bachibouzouk Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk Oct 1, 2020

Choose a reason for hiding this comment

Uh oh!

Bachibouzouk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

mahendrark commented Jul 27, 2020 •

edited

Loading

mahendrark commented Aug 10, 2020 •

edited

Loading

Bachibouzouk left a comment •

edited

Loading

mahendrark commented Oct 1, 2020 •

edited

Loading

Bachibouzouk Oct 1, 2020 •

edited

Loading

Bachibouzouk left a comment •

edited

Loading