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.

Sign up for GitHub

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

Jump to bottom

Fixed bug in EthosBinary dataset class and model directory copying logic in RayTuneReportCallback #1129

Merged
merged 13 commits into from
Mar 25, 2021
Merged

Fixed bug in EthosBinary dataset class and model directory copying logic in RayTuneReportCallback #1129

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f101cda
adding TarDownloadMixin - capability to download + open zipped tar files
ANarayan Mar 15, 2021
d5b3dae
adding header param - can now specify if raw data contains a header i…
ANarayan Mar 15, 2021
5fae49f
adding several text classification datasets
ANarayan Mar 15, 2021
8d06ab2
adding ethos binary dataset
ANarayan Mar 16, 2021
09f171a
fix bug in passing remove_duplicates at train time
ANarayan Mar 16, 2021
ee97a9b
reformat saved ethos binary csv to have comma seperator not semicolon
ANarayan Mar 17, 2021
071304c
casting isHate binary feature to int in EthosBinary
ANarayan Mar 19, 2021
9a9fd0a
adding default value to header param in read_file to avoid errors whe…
ANarayan Mar 19, 2021
e434b6e
merge w/upstream master
ANarayan Mar 19, 2021
76d5e23
fixing RayTune callback checkpoint model copying to be atomic
ANarayan Mar 19, 2021
597aa26
modifying agnews read_file implementation to include header arg
ANarayan Mar 25, 2021
7c244df
fix processed yahoo_answers csv name
ANarayan Mar 25, 2021
c924b9f
add comment to atomic checkpoint copying process
ANarayan Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions ludwig/datasets/ethos_binary/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ class EthosBinary(UncompressedFileDownloadMixin, IdentityProcessMixin,
def __init__(self, cache_dir=DEFAULT_CACHE_LOCATION):
super().__init__(dataset_name="ethos_binary", cache_dir=cache_dir)

def load_processed_dataset(self, split):
dataset_csv = os.path.join(self.processed_dataset_path,
self.csv_filename)
data_df = pd.read_csv(dataset_csv, sep=';')
return data_df
def process_downloaded_dataset(self):
super(EthosBinary, self).process_downloaded_dataset()
# replace ; sperator to ,
processed_df = pd.read_csv(os.path.join(self.processed_dataset_path,
self.csv_filename), sep=";")
# convert float labels (0.0, 1.0) to binary labels
processed_df['isHate'] = processed_df['isHate'].astype(int)
processed_df.to_csv(
os.path.join(self.processed_dataset_path, self.csv_filename),
index=False, sep=","
)
2 changes: 1 addition & 1 deletion ludwig/datasets/mixins/process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class MultifileJoinProcessMixin:
raw_dataset_path: str
processed_dataset_path: str

def read_file(self, filetype, filename, header):
def read_file(self, filetype, filename, header=0):
if filetype == 'json':
file_df = pd.read_json(
os.path.join(self.raw_dataset_path, filename))
Expand Down
10 changes: 9 additions & 1 deletion ludwig/hyperopt/execution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import uuid
import copy
import json
import multiprocessing
Expand Down Expand Up @@ -722,7 +723,14 @@ def on_epoch_end(self, trainer, progress_tracker, save_path):
if trainer.is_coordinator():
with tune.checkpoint_dir(step=progress_tracker.epoch) as checkpoint_dir:
checkpoint_model = os.path.join(checkpoint_dir, 'model')
shutil.copytree(save_path, checkpoint_model)
if not os.path.isdir(checkpoint_model):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a comment here about the fact that the previous implementation was copytree (you can leave that line commented) but that that's not atomic and so we have to do this temp + move thing, otherwise in the future we may forget about it and maybe replace it with copytree again :)

Copy link
Collaborator Author

@ANarayan ANarayan Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

great feedback -- addressed in this commit here: c924b9f

copy_id = uuid.uuid4()
tmp_dst = "%s.%s.tmp" % (checkpoint_model, copy_id)
shutil.copytree(save_path, tmp_dst)
try:
os.rename(tmp_dst, checkpoint_model)
except:
shutil.rmtree(tmp_dst)

train_stats, eval_stats = progress_tracker.train_metrics, progress_tracker.vali_metrics
stats = eval_stats or train_stats
Expand Down