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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial integration with --roboflow_upload #218

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Conversation

SkalskiP
Copy link
Contributor

Hi, @fcakyon 馃憢馃徎!

I spent some time today building the initial version of --roboflow_upload flag. I have a small problem, however.

I'm getting: FileNotFoundError: [Errno 2] No such file or directory: 'runs/train/exp/results.png'. Is there any reason why you decided not to save results.png chart?

opt.data = check_dataset_roboflow(
data=opt.data,
opt.data = RoboflowConnector.download_dataset(
url=opt.data,
roboflow_token=opt.roboflow_token,
task="classify",
location=ROOT.absolute().as_posix()
Copy link
Owner

Choose a reason for hiding this comment

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

it should not download into pip package directory

Copy link
Contributor Author

@SkalskiP SkalskiP Feb 10, 2023

Choose a reason for hiding this comment

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

The current version, with changes I made last time, is downloading to the same directory as the current version. Where would you like me to download it?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, you are right. I missed it the last time. I prefer to save that dataset to the current working directory.

opt.data = check_dataset_roboflow(
data=opt.data,
opt.data = RoboflowConnector.download_dataset(
url=opt.data,
roboflow_token=opt.roboflow_token,
task="segment",
location=ROOT.absolute().as_posix()
Copy link
Owner

Choose a reason for hiding this comment

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

it should not download into pip package directory

opt.data = check_dataset_roboflow(
data=opt.data,
opt.data = RoboflowConnector.download_dataset(
url=opt.data,
roboflow_token=opt.roboflow_token,
task="detect",
location=ROOT.absolute().as_posix()
Copy link
Owner

Choose a reason for hiding this comment

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

it should not download into pip package directory

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

Hi, @fcakyon 馃憢馃徎!

I spent some time today building the initial version of --roboflow_upload flag. I have a small problem, however.

I'm getting: FileNotFoundError: [Errno 2] No such file or directory: 'runs/train/exp/results.png'. Is there any reason why you decided not to save results.png chart?

Actually it should save that file. I did not change anything to skip that file. Is the latest version in original repo saving it?

@SkalskiP
Copy link
Contributor Author

Is the latest version in original repo saving it?

Do you mean YOLOv5? Yes.

@SkalskiP
Copy link
Contributor Author

But you save the result.html file. Is that on purpose?

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

Yes we find it use

Is the latest version in original repo saving it?

Do you mean YOLOv5? Yes.

Then there must be a bug in this repo. I have to check the related line 馃憤

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

But you save the result.html file. Is that on purpose?

Yes we find it very useful in experiment tracking logs.

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

Same line is present in yolov5-pip

ultralytics/yolov5: https://github.com/ultralytics/yolov5/blob/cec1b9bc923cdd235baa3b9b5c80e3700bc9b1dc/utils/loggers/__init__.py#L326-L271

fcakyon/yolov5-pip:

if self.plots:
plot_results(file=self.save_dir / 'results.csv') # save results.png

@SkalskiP
Copy link
Contributor Author

@fcakyon it is about results.png, not results.csv. The .csv file was saved properly.

@SkalskiP
Copy link
Contributor Author

Yes we find it very useful in experiment tracking logs.

So it is an additional file, not instead of result.png?

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

Yes we find it very useful in experiment tracking logs.

So it is an additional file, not instead of result.png?

Yes it is an addition to results.png.

@fcakyon
Copy link
Owner

fcakyon commented Feb 10, 2023

@fcakyon it is about results.png, not results.csv. The .csv file was saved properly.

Check the comment in the line I provided. plot_results function reads values from results.csv and then exports the results as results.png.

def plot_results(file='path/to/results.csv', dir=''):
# Plot training results.csv. Usage: from yolov5.utils.plots import *; plot_results('path/to/results.csv')
save_dir = Path(file).parent if file else Path(dir)
fig, ax = plt.subplots(2, 5, figsize=(12, 6), tight_layout=True)
ax = ax.ravel()
files = list(save_dir.glob('results*.csv'))
assert len(files), f'No results.csv files found in {save_dir.resolve()}, nothing to plot.'
for f in files:
try:
data = pd.read_csv(f)
s = [x.strip() for x in data.columns]
x = data.values[:, 0]
for i, j in enumerate([1, 2, 3, 4, 5, 8, 9, 10, 6, 7]):
y = data.values[:, j].astype('float')
# y[y == 0] = np.nan # don't show zero values
ax[i].plot(x, y, marker='.', label=f.stem, linewidth=2, markersize=8)
ax[i].set_title(s[j], fontsize=12)
# if j in [8, 9, 10]: # share train and val loss y axes
# ax[i].get_shared_y_axes().join(ax[i], ax[i - 5])
except Exception as e:
LOGGER.info(f'Warning: Plotting error for {f}: {e}')
ax[1].legend()
fig.savefig(save_dir / 'results.png', dpi=200)
plt.close()

@fcakyon
Copy link
Owner

fcakyon commented Mar 4, 2023

@SkalskiP is there anything I can do for this PR?

I will be very busy with a conference submission deadline till next Friday but can help you with this PR after then 馃憤

@SkalskiP
Copy link
Contributor Author

SkalskiP commented Mar 6, 2023

@fcakyon, no worries. I think it is mostly on me to figure it out. As a matter of fact, I'm working on it now.

@SkalskiP
Copy link
Contributor Author

SkalskiP commented Mar 6, 2023

@fcakyon calling plot_results as you suggested solved my problem. Upload works. Tests fail as I don't have access to your secret Roboflow key. When you run them, they should pass. 馃煝 馃И 馃馃徎

@SkalskiP
Copy link
Contributor Author

@fcakyon are you back from the conference already?

@fcakyon
Copy link
Owner

fcakyon commented Mar 13, 2023

Yes, I am back home, I will look into this as soon as possible. Sorry for the delay.

@SkalskiP
Copy link
Contributor Author

@fcakyon, no worries :)

@fcakyon fcakyon merged commit 9f170f3 into fcakyon:main Mar 20, 2023
@fcakyon
Copy link
Owner

fcakyon commented Mar 20, 2023

@SkalskiP it passed the tests. Thank you for the PR!

@SkalskiP
Copy link
Contributor Author

@fcakyon, is it already released?

@SkalskiP
Copy link
Contributor Author

@fcakyon okay, I checked. It looks like it is not part of 7.0.10 release. Any idea when you plan to release roboflow_upload?

@fcakyon
Copy link
Owner

fcakyon commented Mar 21, 2023

@SkalskiP
Copy link
Contributor Author

@fcakyon let me test that

@SkalskiP
Copy link
Contributor Author

@fcakyon works! 馃帀

@fcakyon
Copy link
Owner

fcakyon commented Mar 21, 2023

@fcakyon works! 馃帀

Awesome news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants