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

Using the old-fashioned input_file_x.py does not work #83

Closed
Stevogallo opened this issue Oct 4, 2023 · 12 comments · Fixed by #102
Closed

Using the old-fashioned input_file_x.py does not work #83

Stevogallo opened this issue Oct 4, 2023 · 12 comments · Fixed by #102
Assignees
Labels
bug Something isn't working

Comments

@Stevogallo
Copy link
Collaborator

Actually mirroring what has been raised in Issue-81
When I try to use the input file in .py format, it works with the 3 embedded examples, but if I create an input_file_x.py with x=/=[1,2,3] it says module ramp_something not found.

Furthermore, if I try to modify the content of input_file_1/2/3.py and run ramp_run, it still simulates follwing the embedded example, ignoring the modification.

@FLomb FLomb self-assigned this Oct 4, 2023
@FLomb FLomb added the bug Something isn't working label Oct 4, 2023
@FLomb
Copy link
Contributor

FLomb commented Oct 4, 2023

Thanks for spotting this issue. This is similar to issue #81, but originating from a different line of the code since you are not converting .py files into .xlsx as in this prior, related issue.

I believe the issue here lies in line 29 of the initialise.py script, where the code tries to convert the file index (j in the code) you have specified, e.g., 1,2,3 or 4, etc., into a full path to a file. In fact, what happens is the following:
file_module = importlib.import_module(f"ramp.example.input_file_{j}").

Now, if ramp is not an installed package on your conda environment, the import string, which becomes, e.g.: "ramp.example.input_file_1", is correctly interpreted as a request to import the python file within your ramp folder. But, as per the documentation, you are supposed to run RAMP within your ramp environment, which includes the ramp package and the ramp.example functionality. So, if ramp is an installed package in your conda environment, the same string ("ramp.example.input_file_1") gets interpreted as "give me the pre-defined input_file_1 as originally uploaded on pip", which will not include any changes you made locally. And if, instead, you try to ask for a different file, e.g., "input_file_4", this leads to an error because there are only 1,2 and 3 examples on pip.

This can be easily fixed in various ways, but I wonder which one is the best. Perhaps, @Bachibouzouk , we should get rid of ramp as a package name and use rampdemand, which would avoid ambiguity with the /ramp/ folder?

@FLomb
Copy link
Contributor

FLomb commented Oct 4, 2023

For further clarity on what I meant above, for instance, you may run help(ramp.example), which shows the following:


help(ramp.example)
Help on package ramp.example in ramp:

NAME
    ramp.example

PACKAGE CONTENTS
    examples
    input_file_1
    input_file_2
    input_file_3

FILE
    /home/fl/GitHub-repos/RAMP/ramp/example/__init__.py

So the code basically keeps looking for these 3 pre-defined examples and ignores anything happening locally

@Bachibouzouk
Copy link
Collaborator

Bachibouzouk commented Oct 4, 2023

A solution to keep the old way could be to use another way to import a module

@Bachibouzouk
Copy link
Collaborator

A solution to keep the old way could be to use another way to import a module

I could not make it work :(

@FLomb
Copy link
Contributor

FLomb commented Oct 26, 2023

Have we decided that the option to get rid of ramp as a package name and use rampdemand to avoid ambiguity with the /ramp/ folder would lead to too much trouble in the code?

@Bachibouzouk
Copy link
Collaborator

Bachibouzouk commented Oct 26, 2023

I think the way to go around this difficulty would be to add a block of code at the end of the file which does create a UseCase with the user_list and then call a method to simulate daily profiles
for example :

if __name__=="__main__":
    from ramp.core.core import UseCase
    # date_start and date_end are optional and a full year would be automatically picked if not provided
    uc = UseCase(users=User_list,date_start=...,date_end=...)
    # This would encompass running calc_peak_time_range() as a method of UseCase instance and
    # initialise_inputs() would not really need to be called as all it does is return peak_enlarge = 0.15,
    # the profile number and the user_list which we already have here...
    uc.initialize(num_profile=10, peak_enlarge=0.15)

    # this should a new method which would call generate_daily_load_profiles() for each day of day_types
    uc.generate_load_profiles(parallel=False, day_types=...)
    # this would be a new method using work of @mohammadamint 
    results = uc.export_results()

Alternatively we can also modify ramp/run_usecase.py to accept user_list instead of j for the integer corresponding to the input userfile

if __name__=="__main__":
    from ramp.ramp_run import run_usecase
    # here get rid of j as argument and replace it by a user_list
    run_usecase(user_list=user_list, num_profiles=None, days=None, plot=True, parallel=False)
 

Both solution highlighted there would fit to users who like to run ramp from IDE and not from terminal, it would also work from terminal with python <name of input file>

The first solution would help greatly to make maintainability easier for .xlsx users, pure python users in IDE and in Jupyter notebooks so I would vote for this one :)

@Bachibouzouk
Copy link
Collaborator

What do you think @FLomb, @mohammadamint ?

@FLomb
Copy link
Contributor

FLomb commented Nov 6, 2023

I think the first solution would work nicely. My only concern is about how many functions named something like generate_(some-kind-of)_load_profile(s) we would end up having. So far, we already have:

  • generate_load_profile
  • generate_daily_load_profiles
  • generate_daily_load_profiles_parallel
  • generate_single_load_profile
  • generate_aggregated_load_profile

The new method you propose, generate_load_profiles, would have a similar enough name to the existing ones. Can we come up with something more specific? Or, do we need a new method at all? Can't we just use generate_daily_load_profiles(_parallel), as it happens in stochastic_process.py?

@mohammadamint
Copy link
Collaborator

I think the way to go around this difficulty would be to add a block of code at the end of the file which does create a UseCase with the user_list and then call a method to simulate daily profiles for example :

if __name__=="__main__":
    from ramp.core.core import UseCase
    # date_start and date_end are optional and a full year would be automatically picked if not provided
    uc = UseCase(users=User_list,date_start=...,date_end=...)
    # This would encompass running calc_peak_time_range() as a method of UseCase instance and
    # initialise_inputs() would not really need to be called as all it does is return peak_enlarge = 0.15,
    # the profile number and the user_list which we already have here...
    uc.initialize(num_profile=10, peak_enlarge=0.15)

    # this should a new method which would call generate_daily_load_profiles() for each day of day_types
    uc.generate_load_profiles(parallel=False, day_types=...)
    # this would be a new method using work of @mohammadamint 
    results = uc.export_results()

Alternatively we can also modify ramp/run_usecase.py to accept user_list instead of j for the integer corresponding to the input userfile

if __name__=="__main__":
    from ramp.ramp_run import run_usecase
    # here get rid of j as argument and replace it by a user_list
    run_usecase(user_list=user_list, num_profiles=None, days=None, plot=True, parallel=False)
 

Both solution highlighted there would fit to users who like to run ramp from IDE and not from terminal, it would also work from terminal with python <name of input file>

The first solution would help greatly to make maintainability easier for .xlsx users, pure python users in IDE and in Jupyter notebooks so I would vote for this one :)

I would rather have the first solution.

@Bachibouzouk
Copy link
Collaborator

The new method you propose, generate_load_profiles, would have a similar enough name to the existing ones. Can we come up with something more specific? Or, do we need a new method at all? Can't we just use generate_daily_load_profiles(_parallel), as it happens in stochastic_process.py?

Sorry @FLomb it was a typo on my side, I meant already existing generate_daily_load_profiles from UseCase class. I wrote this pseudo code on the fly and I copy-pasted the wrong name of the method!

To me it seems we all agree to the first proposition, i.e.

if __name__=="__main__":
    from ramp.core.core import UseCase
    # date_start and date_end are optional and a full year would be automatically picked if not provided
    uc = UseCase(users=User_list,date_start=...,date_end=...)
    # This would encompass running calc_peak_time_range() as a method of UseCase instance and
    # initialise_inputs() would not really need to be called as all it does is return peak_enlarge = 0.15,
    # the profile number and the user_list which we already have here...
    uc.initialize(num_profile=10, peak_enlarge=0.15)

    # this should a new method which would call generate_daily_load_profiles() for each day of day_types
    uc.generate_daily_load_profiles(parallel=False, day_types=...)
    # this would be a new method using work of @mohammadamint 
    results = uc.export_results()

@FLomb
Copy link
Contributor

FLomb commented Nov 7, 2023

Ok great, then yeah, let's proceed with the first proposition!

@FLomb FLomb mentioned this issue Nov 9, 2023
@Bachibouzouk Bachibouzouk linked a pull request Nov 10, 2023 that will close this issue
@FLomb
Copy link
Contributor

FLomb commented Dec 6, 2023

This issue was addressed by the new release v0.5.0. FYI, @Stevogallo, if you want to try out the new version (check out the updated README file and docs).

@FLomb FLomb closed this as completed Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants