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 base load analysis module with data validation and metrics calculation #17

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

Molier
Copy link
Collaborator

@Molier Molier commented Nov 29, 2024

Introduce a new module for base load analysis, including data validation, metrics calculation, and support for different timeframes. Add test cases to ensure functionality and include visualizations for better insights.

@JrtPec
Copy link
Contributor

JrtPec commented Dec 6, 2024

  1. The initial portion of demo_baseLoad.ipynb focuses on experimenting with a different JSON format for efficient data handling. While this is potentially valuable, it’s not directly related to the baseload analysis and might be better placed in its own notebook and PR.

  2. Consider adding more structure to demo_baseLoad.ipynb:

    • Introduction: Briefly describe the notebook’s purpose and what will be demonstrated.
    • Data Loading: Show how to load the input data (e.g., into a DataFrame), and explain the required input format so users can apply their own data.
    • Analysis Execution: Make it straightforward to run the analysis by defining a few parameters and calling the analysis function.
    • Results Presentation: Display the raw output of the analysis.
    • Visualization: Include clear visualizations to help interpret the results.
  3. The main function currently takes a file path as input. Consider splitting it into two separate functions: one for loading data into a DataFrame (or another appropriate structure), and another for performing the actual analysis.

  4. You’ve introduced a TimeFrame Enum, but there’s already a Granularity Enum in enums.py. Is it possible to reuse Granularity here, or is there a specific reason for introducing a new TimeFrame Enum?

  5. The sample data used here suggests a daily usage of around 2176 kWh and a baseload of approximately 90 kW. This seems unusually high for a typical building. Could you clarify the building type or confirm that the units are correct?

…ed up base_load demo to be neat and packaged. chore: added some performance testing.
…seperation.

feat: added some more explanation to the base_load demo and some extra analysis, made sure all the graphs worked, differnce between large building and residential...
@Molier
Copy link
Collaborator Author

Molier commented Dec 9, 2024

  1. The initial portion of demo_baseLoad.ipynb focuses on experimenting with a different JSON format for efficient data handling. While this is potentially valuable, it’s not directly related to the baseload analysis and might be better placed in its own notebook and PR.

  2. Consider adding more structure to demo_baseLoad.ipynb:

    • Introduction: Briefly describe the notebook’s purpose and what will be demonstrated.
    • Data Loading: Show how to load the input data (e.g., into a DataFrame), and explain the required input format so users can apply their own data.
    • Analysis Execution: Make it straightforward to run the analysis by defining a few parameters and calling the analysis function.
    • Results Presentation: Display the raw output of the analysis.
    • Visualization: Include clear visualizations to help interpret the results.
  3. The main function currently takes a file path as input. Consider splitting it into two separate functions: one for loading data into a DataFrame (or another appropriate structure), and another for performing the actual analysis.

  4. You’ve introduced a TimeFrame Enum, but there’s already a Granularity Enum in enums.py. Is it possible to reuse Granularity here, or is there a specific reason for introducing a new TimeFrame Enum?

  5. The sample data used here suggests a daily usage of around 2176 kWh and a baseload of approximately 90 kW. This seems unusually high for a typical building. Could you clarify the building type or confirm that the units are correct?

All raised questions should be touched upon in the new commits.

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.

2 participants