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

Remove tutorials #156

Merged
merged 11 commits into from
Feb 8, 2023
Merged

Remove tutorials #156

merged 11 commits into from
Feb 8, 2023

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Nov 27, 2022

After FluxML/Flux.jl#2125 I believe we can delete the tutorial section here completely. Then we have code in two places, the docs + the model zoo, not three. As desired for instance here: #141 (comment)

Besides the 5 moved to docs already:

This PR also tries to clean up the navbar. The code had a baroque set of switches apparently to deal with relative paths from various places. I'm not certain what it should contain, but the first attempt is this (with current state on top, PR below):

Screenshot 2022-11-26 at 23 33 16

At present that has no links to docs sub-sections. But perhaps it ought not to link to stack overflow / discourse, those are just links not things Flux owns. And could then instead have Ecosystem?

Edit, I see that #140 is also quite recent WIP. But this aims for the blog here, not the tutorials, I think.

Closes #122
Closes #132

@github-actions
Copy link

Once the build has completed, you can preview your PR at this URL: https://fluxml.ai/previews/PR156/

@mcabbott
Copy link
Member Author

Attempting to add redirects in last commit. But https://fluxml.ai/previews/PR156/getting_started/ doesn't work, although the file looks like https://github.com/FluxML/fluxml.github.io/blob/main/blogposts/2017-08-24-generic-gpu.md which does redirect. Is this a preview limitation?

@mcabbott
Copy link
Member Author

Since Flux 0.13.9 is tagged, this ought to be safe to merge.

However, it would be nice if the redirection worked.

I'm also not sure they point to the right place. Maybe some should point to the model zoo instead.

@Saransh-cpp
Copy link
Member

The code had a baroque set of switches apparently to deal with relative paths from various places.

This code is a bit annoying, but unfortunately, this is what keeps the previews working 🙂

Given that we store previews within the gh-pages branch (to avoid the extra Netlify infrastructure), it becomes important to ensure that the previews don't access the content of the stable website and vice-versa.

@Saransh-cpp
Copy link
Member

For instance, if you go to the preview and click the "blog" button on the navbar, it will redirect you to the "blog" section of the stable website (because this PR now does not use relative links).

@mcabbott
Copy link
Member Author

mcabbott commented Dec 3, 2022

I see. Is there any way to do that without duplicating the content? Define some variable which is either "" or "../" at the top of the page & splice that in?

Otherwise having to typing the right path into the online preview (but not local ones) might still be preferable to having multiple copies of the same content which can have different mistakes...

@Saransh-cpp
Copy link
Member

Is there any way to do that without duplicating the content?

I'll try that! Stuck with my end semester examinations at the moment; we could either switch back to the weird if-else blocks, or keep it like this (as you suggest) for this PR.

@Saransh-cpp
Copy link
Member

Attempting to add redirects in last commit. But https://fluxml.ai/previews/PR156/getting_started/ doesn't work, although the file looks like https://github.com/FluxML/fluxml.github.io/blob/main/blogposts/2017-08-24-generic-gpu.md which does redirect. Is this a preview limitation?

Not very sure what is happening here. It should've worked. Will look into this.

@mcabbott
Copy link
Member Author

mcabbott commented Dec 3, 2022

If restored, the if-else block need only wrap a few entries like blog, rather than duplicating everything.

You don't have any ideas why the redirects are failing, by the way?

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Dec 3, 2022

As far as I can see, the GH Actions are building the redirected endpoints - https://github.com/FluxML/fluxml.github.io/tree/gh-pages/previews/PR156.

But, no idea why the deployment is not showing them.

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Dec 3, 2022

Oh wait, I think the redirects do not happen automatically, rather - https://github.com/FluxML/fluxml.github.io/blob/main/utils.jl - is used to make everything work.

But the endpoints should still work.

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Dec 3, 2022

Ah! This PR tries to redirect some pages, but there is no default redirect (or external) command. I went through Franklin's documentation and found nothing on it.

The GH Actions are producing the HTML files, but these HTML files are empty (as the markdown files are empty; Franklin does not understand external). Thus, we do get the endpoints, but these endpoints are blank pages.

The website, right now, uses utils.jl to provide redirects for blogs and tutorials (utils.jl understands external). We should write a similar script for these lost pages, or we could copy paste the pages from Flux's documentation and the endpoints will work.

(Not preview specific, the redirects will fail on the stable website too.)

@mcabbott
Copy link
Member Author

Are you saying that hfun_recentposts in utils.jl is run only on the blog posts (whose redirects via external seem to work) and not on the tutorialposts?

@Saransh-cpp
Copy link
Member

I might have explained the existing mechanism wrong. Technically, no "redirection" is involved in the blog posts and tutorials. The function hfun_recentposts is responsible for writing the https://fluxml.ai/blog/ and https://fluxml.ai/tutorials/ pages. The links on these pages are given the external variable's value.

For instance, if you go to https://fluxml.ai/blogposts/2018-12-03-ml-language-compiler/ (blog hosted somewhere else), the website will not redirect you. You will be shown an empty web page because the markdown file is empty. But, if you click on "Building a Language and Compiler for Machine Learning (December 2018)" on the "blogs" page, you will land on the correct URL (not redirection, the link itself is the link where the blog is hosted).

@mcabbott
Copy link
Member Author

Oh right, sorry, I see now.

Ok, so copying that isn't really the problem. We don't need a page here which links to the new locations.

The only reason to want redirects is in case people elsewhere have links to the existing pages. (Or google might.) Such redirects would have to be made by a different mechanism.

@Saransh-cpp
Copy link
Member

I'll check on Franklin's GitHub or Slack if there is a nice way to do this.

_layout/navbar.html Outdated Show resolved Hide resolved
@mcabbott
Copy link
Member Author

Bump -- https://twitter.com/owainkenway/status/1619003403765694465 is a recent complaint which I'm told is about old Flux tutorials being misleading.

@Saransh-cpp
Copy link
Member

My message seems to have gone up on slack. I hope tagging @tlienart here would be okay.

Just to reiterate, we want to redirect users from some specific endpoint to another. For instance, https://clusterinnovationcentre.github.io will automatically redirect you to https://clusterinnovationcentre.github.io/convoke/2023. This happens because index.html has the following contents -

<meta http-equiv="refresh" content="0; url=./2023/"/>

Can we perform such a redirect (another website redirect instead of a relative URL redirect) using Franklin? Maybe something like a "redirect" variable?

@tlienart
Copy link
Contributor

No problem, assuming you're using Franklin and not Xranklin (I'm on my phone so can't check) you can use {{redirect the/path/you/want.html}} on a target page. Someone entering the path you want as URL would then be redirected to the target page

@Saransh-cpp
Copy link
Member

Thank you! That should do!

@mcabbott
Copy link
Member Author

149a8bb adds something like that. But going to https://fluxml.ai/previews/PR156/tutorialposts/2021-10-14-vanilla-gan/ still loads a stub page here, rather than a page in the docs.

@tlienart
Copy link
Contributor

tlienart commented Jan 31, 2023

You should remove the first part up to the path prefix (so everything before fluxml and including it) + add an explicit path all the way to .html (see here: https://franklinjl.org/syntax/page-variables/index.html#basic_functions) in your case it should be something like

{{redirect /tutorials/.../index.html}}

where the ... is replaced with what you have in your commit after tutorials

Copy link
Contributor

@tlienart tlienart left a comment

Choose a reason for hiding this comment

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

the /stable/ is added automatically by your deploy script I think so you should remove that as well otherwise this is basically /stable/stable/...

@mcabbott
Copy link
Member Author

Thanks for taking a look.

But is this the right way around? The linked docs say "when a user goes to (baseurl)/some/other/addr.html, they will be redirected to the current page" which sound like this code should be on the new page (where you want readers to land).

The meta tag above is instead placed on the old page (where you're removing content) to send readers elsewhere.

The pages we would ideally like to send readers to are either in Flux's docs (generated by Documenter) or else in the model-zoo, a separate repository with README pages.

@tlienart
Copy link
Contributor

tlienart commented Jan 31, 2023

Ah crap I misunderstood your problem it seems, what you put in your comment is right:

with the {{redirect /some/path/index.html}} if you put it on foo/bar/baz.md then

  • the target page is /foo/bar/baz/
  • if a user goes to /foo/bar/baz/ they'll stay there
  • if a user goes to /some/path/ they'll go to /foo/bar/baz/

I think that was clear from your previous comment.

Now if you want the reverse, i.e. you put the content on /foo/bar/baz.md but instead of wanting the content at /foo/bar/baz/ you want it at /some/path/ then you want to add

+++
... # other definitions here 
slug = "/some/path/"
+++

at the top of /foo/bar/baz.md.

In that case when a user goes to /some/path/ they'll stay there and see the content translated from /foo/bar/baz.md

I hope this clears things up a bit for your use case.

@mcabbott
Copy link
Member Author

Ok I think I understand. But the point of this PR is no longer to attempt to host this code within the website at all (since it goes stale here).

The hope was to send anyone following a link here along to either the model zoo (different domain, github README) or else to a page in the docs (same domain, Documenter). I believe that's what Saransh's meta code would do -- there isn't some way to embed a literal tag?

Or can we use .htaccess here?

@tlienart
Copy link
Contributor

tlienart commented Jan 31, 2023

this is getting a bit tricky but doable, if you want all pages to redirect to some url (say URL) in your layout you can add something like

<head>
...
{{ispage tutorials/*}}
  <meta http-equiv="Refresh" content="0; url='URL'" />
{{end}}
...
</head>

any page generated under tutorials will then act as a redirect to the same point.

if you want each page to act as a redirect to a separate location, you can code this up in a utils.jl function and call it in <head></head>

@tlienart
Copy link
Contributor

tlienart commented Jan 31, 2023

(comment updated, meta refresh needs to be in <head> so you can't just put that in the body)

Copy link
Member

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. I would also remove "Discourse / Stack Overflow" for a link to "Ecosystem." The latter should be front and center IMO since we get a lot of people making feature requests or posts elsewhere for things that are in existing (not Flux.jl) packages.

@mcabbott
Copy link
Member Author

mcabbott commented Feb 8, 2023

Great, the present state of this PR has these headings:

Screenshot 2023-02-08 at 14 12 30

I am giving up on making redirects work; someone else can try (in another PR) if strongly desired. The main thing is not to leave bad & outdated examples up in such an official place.

@mcabbott
Copy link
Member Author

mcabbott commented Feb 8, 2023

Ah in fact https://fluxml.ai/previews/PR156/tutorialposts/2021-10-14-vanilla-gan/ does now redirect. I don't know what changed. But 1d13aa4 could then be done for other pages too. In another PR.

@mcabbott mcabbott merged commit e0ed181 into main Feb 8, 2023
@mcabbott mcabbott deleted the rm-tutorials branch February 8, 2023 19:16
@darsnack
Copy link
Member

darsnack commented Feb 8, 2023

Weird, I didn't see it in the preview earlier today, but I see it now.

@mcabbott
Copy link
Member Author

mcabbott commented Feb 8, 2023

Weird indeed.

Thanks @tlienart for your help with this!

@Saransh-cpp Saransh-cpp mentioned this pull request Oct 30, 2023
2 tasks
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.

getting started webpage error Simple multi-layer perceptron tutorial does not work on macOS Monterey
4 participants