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

Update diagram #483

Merged
merged 9 commits into from
Oct 4, 2023
Merged

Update diagram #483

merged 9 commits into from
Oct 4, 2023

Conversation

PierreGtch
Copy link
Collaborator

incorporate cache in diagram #439

@sylvchev I slightly changed the diagram to put the paradigm between the dataset and the evaluation. What do you think?

@bruAristimunha
Copy link
Collaborator

Hi @PierreGtch,

In the github, it’s too close
image

I would suggest going beyond what we have at the moment:

Random examples:
https://miro.medium.com/v2/resize:fit:4800/format:webp/1*tWzaMziYprMPGThHaJuw7w.png
https://camo.githubusercontent.com/afa74028422ebd312a7b11e7bc037342e4c689c1fa47240391305f8de928a0bc/687474703a2f2f646174612e64676c2e61692f61737365742f696d6167652f44474c2d417263682e706e67
https://www.google.com/url?sa=i&url=https%3A%2F%2Fse.ewi.tudelft.nl%2Fdesosa2019%2Fchapters%2Fpytorch%2F&psig=AOvVaw3ssHzHSDxqxh8lN5kx0BrT&ust=1695139827677000&source=images&cd=vfe&opi=89978449&ved=2ahUKEwjxrrqLxrSBAxWmmycCHaJrB5kQjRx6BAgAEAw
https://raw.githubusercontent.com/mrdbourke/pytorch-deep-learning/main/images/01-machine-learning-a-game-of-two-parts.png
https://raw.githubusercontent.com/mrdbourke/pytorch-deep-learning/main/images/01_a_pytorch_workflow.png

bruAristimunha
bruAristimunha requested changes Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@bruAristimunha bruAristimunha left a comment

Choose a reason for hiding this comment

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

Almost :)

docs/source/images/M.svg Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change the logo?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks different in the github

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At some point I wanted to include the logo in the diagram so I needed a vectorized version of the logo.

But the font is not correct on your screenshot, it is supposed to be a simple copy of the current logo. Strange because the font was supposed to be embedded in the SVG...

docs/source/images/architecture.svg Show resolved Hide resolved
@bruAristimunha
Copy link
Collaborator

Personally, I wouldn't say I like the aesthetics of the colours and composition of the items, but this is not related to its PR. Perhaps it can be resolved with the PR.

@bruAristimunha
Copy link
Collaborator

@sylvchev, can you take a quick look too?

@PierreGtch
Copy link
Collaborator Author

Personally, I wouldn't say I like the aesthetics of the colours and composition of the items, but this is not related to its PR. Perhaps it can be resolved with the PR.

haha we can change that, I took a quite generic palette of draw.io

@PierreGtch
Copy link
Collaborator Author

As there are some issues with the fonts, here is rendered look:
image

@PierreGtch PierreGtch marked this pull request as ready for review October 2, 2023 12:25
@bruAristimunha
Copy link
Collaborator

It's great now @PierreGtch! I will merge, thank you again =)

@bruAristimunha bruAristimunha enabled auto-merge (squash) October 3, 2023 23:11
@bruAristimunha bruAristimunha merged commit 756c00f into NeuroTechX:develop Oct 4, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sylvchev sylvchev Awaiting requested review from sylvchev

@bruAristimunha bruAristimunha Awaiting requested review from bruAristimunha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PierreGtch @bruAristimunha