Skip to content

Cross validation#6

Open
ebensonm wants to merge 91 commits intomasterfrom
cross_validation
Open

Cross validation#6
ebensonm wants to merge 91 commits intomasterfrom
cross_validation

Conversation

@ebensonm
Copy link
Copy Markdown
Contributor

Cross validation branch. The relevant code is under cross_validation_mpi.py

Comment thread d3m_profiler/embed.py Outdated
Comment thread example_mpi.py Outdated
Comment thread naive_mpi.py Outdated
Comment thread save_embed.py Outdated
Comment thread save_embed_elmo.py Outdated
Comment thread cross_validation_mpi.py Outdated
Comment thread cross_validation_mpi.py Outdated
Comment on lines +77 to +78
del X_train
del y_train
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this will happen automatically with python garbage collection when this function returns.

Comment thread save_embed.py Outdated
Copy link
Copy Markdown

@e13h e13h left a comment

Choose a reason for hiding this comment

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

I don't understand much of what's going on with the multiprocessing, but I noticed that there is code for it in get_variables(), evaluate_model(), run_models(), and embed_data(). I just wonder if there's a way to encapsulate it in such a way that the setting up and divvying out of jobs is in one function and running of models, data loading, etc. doesn't have to have any knowledge of multiprocessing.

Other than that, I have made some suggestions about changing how we pass data to the embed() function so that it is a little less clunky.

Comment thread d3m_profiler/evaluate_models.py Outdated
Comment thread d3m_profiler/embed.py Outdated
Comment thread d3m_profiler/embed.py Outdated
Comment thread d3m_profiler/embed.py Outdated
Comment thread d3m_profiler/embed.py Outdated
Comment thread d3m_profiler/embed.py Outdated
Comment thread save_embed.py Outdated
Comment thread requirements.txt
Comment thread requirements.txt
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