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 parameter "progress file" #1038

Merged
merged 13 commits into from
Apr 10, 2020

Conversation

Jaimecclin
Copy link
Contributor

[please review the Contribution Guidelines prior to submitting your pull request. go ahead and delete this line if you've already reviewed said guidelines.]

What does this PR do?

Add a new parameter to output tqdm progress to a file.

Where should the reviewer start?

tpot/base.py

How should this PR be tested?

  1. Test on progress_file as None
  2. Test on progress_file as file_handler

Any background context you want to provide?

What are the relevant issues?

[you can link directly to issues by entering # then the number of the issue, for example, #3 links to issue 3]

Screenshots (if appropriate)

Questions:

  • Do the docs need to be updated?
    api.md is already updated.
  • Does this PR add new (Python) dependencies?
    Nope.

@coveralls
Copy link

coveralls commented Apr 3, 2020

Coverage Status

Coverage increased (+0.004%) to 96.763% when pulling 36c38ab on Jaimecclin:add_progress_file into 6306e60 on EpistasisLab:development.

Copy link
Contributor

@weixuanfu weixuanfu left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Could you please provide small examples (with population=20, generations=10) of this log file when verbosity=1, 2, 3?

@@ -232,7 +233,8 @@ def __init__(self, generations=100, population_size=100, offspring_size=None,
A setting of 2 or higher will add a progress bar during the optimization procedure.
disable_update_check: bool, optional (default: False)
Flag indicating whether the TPOT version checker should be disabled.

progress_file: io.TextIOWrapper or io.StringIO, optional (defaul: sys.stdout)
Save progress content to a file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please rename the progress_file to log_file? I think there are some log infos when verbosity=3.

tpot/base.py Outdated Show resolved Hide resolved
@Jaimecclin
Copy link
Contributor Author

Jaimecclin commented Apr 3, 2020

Thank you for the PR. Could you please provide small examples (with population=20, generations=10) of this log file when verbosity=1, 2, 3?

Of course I can. I am wondering that I should write an example and take screenshots or write an unit test. How do you think?

@weixuanfu
Copy link
Contributor

unit tests are preferred.

@Jaimecclin
Copy link
Contributor Author

Jaimecclin commented Apr 4, 2020

Hi,
I added a file to test verbosity from 1-3. In case of verbosity as 3, an error was raised. It seems it uses xgboost when verbosity is 3.

Traceback (most recent call last): File "/app/tpot/operator_utils.py", line 80, in source_decode exec('from {} import {}'.format(import_str, op_str)) File "<string>", line 1, in <module> ModuleNotFoundError: No module named 'xgboost'

I am not sure the reason so I just commented out the last unit test. And it also happens in master branch. @weixuanfu Could you please help to answer this? If it's an issue, I can try to figure out it.

@Jaimecclin
Copy link
Contributor Author

Jaimecclin commented Apr 5, 2020

Hi,
I added a file to test verbosity from 1-3. In case of verbosity as 3, an error was raised. It seems it uses xgboost when verbosity is 3.

Traceback (most recent call last): File "/app/tpot/operator_utils.py", line 80, in source_decode exec('from {} import {}'.format(import_str, op_str)) File "<string>", line 1, in <module> ModuleNotFoundError: No module named 'xgboost'

I am not sure the reason so I just commented out the last unit test. And it also happens in master branch. @weixuanfu Could you please help to answer this? If it's an issue, I can try to figure out it.

Hi,

I found the reason why it raise exception only in case of verbosity as 3.

    tmp_path = sourcecode.split('.')
    op_str = tmp_path.pop()
    import_str = '.'.join(tmp_path)
    try:
        if sourcecode.startswith('tpot.'):
            exec('from {} import {}'.format(import_str[4:], op_str))
        else:
            exec('from {} import {}'.format(import_str, op_str))
        op_obj = eval(op_str)
    except Exception as e:
        if verbose > 2:
            raise ImportError('Error: could not import {}.\n{}'.format(sourcecode, e))
        else:
            print('Warning: {} is not available and will not be used by TPOT.'.format(sourcecode))
        op_obj = None

    return import_str, op_str, op_obj

I think it can be modified as printing warning simply instead of raising error. I can put my unit test 3 back as well. Any suggestion is welcome. :)

@weixuanfu
Copy link
Contributor

Thank you for looking into it.

The issue is that there is no xgboost in the test environment. We intentionally let tpot raise that import error when verbosity=3.

I think in the CI envs of TPOT unit tests (Linux and Windows), xgboost should be installed so that this error should not be raised.

Could you please build a similar conda env with xgboost for testing unit test with verbosity=3?

@Jaimecclin
Copy link
Contributor Author

Jaimecclin commented Apr 9, 2020

Ok. I tried it and passed all tests. Please take a look. Thanks.

@weixuanfu weixuanfu changed the base branch from master to development April 9, 2020 13:04
Copy link
Contributor

@weixuanfu weixuanfu left a comment

Choose a reason for hiding this comment

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

Thank you for updating. I just found a few minor farther changes. I will merge it to dev branch first for next version of TPOT once those changes are made.

tests/test_progress_parameter.py Outdated Show resolved Hide resolved
tpot/base.py Outdated Show resolved Hide resolved
@Jaimecclin
Copy link
Contributor Author

Hi,
Should I resolve that conflict in base.py?

@weixuanfu
Copy link
Contributor

Hi,
Should I resolve that conflict in base.py?

I have resolved it. Thank you!

@weixuanfu
Copy link
Contributor

Thank you for the PR. I will merge it to dev branch.

@weixuanfu weixuanfu merged commit 298049c into EpistasisLab:development Apr 10, 2020
@Jaimecclin
Copy link
Contributor Author

Thanks for your help, @weixuanfu. :)

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.

3 participants