Skip to content

Conversation

@fabian-kutschera
Copy link
Contributor

@Aangniu here is the preliminary GitHub repo, which we could use for the Quakeworx workshop.

I added the files incl. the meshes you sent me for tpv13 and kaikoura (LSW), but still need to adjust and upload the corresponding notebooks.

Please create a folder for your non-linear example.

Copy link
Contributor

@Thomas-Ulrich Thomas-Ulrich left a comment

Choose a reason for hiding this comment

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

I would use symbolic links as much as possible to the current repo.
does not make sense to duplicate large binary files on a github repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

but this file is already in training/kaikoura/ASAGI/

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, Thomas! Thanks for the suggestion. One thing that may be easier for the Quakworx specifically is that we would like to ask the audience to download these input files locally on their laptop. Then, we will ask them to upload them to the website. So, it would be easier to explain if all the files are inside the same folder.

Maybe it is an option to keep the duplicated files here until the training is done and then we change this to a link?

@Aangniu Aangniu assigned Aangniu and unassigned Aangniu Jan 15, 2025
@Aangniu
Copy link
Contributor

Aangniu commented Jan 15, 2025

Just added the jupyter notebook for post processing and the input files for the cdb_tpv23 example.

@AliceGabriel
Copy link
Collaborator

We are a bit in a time crunch for this one. The SeisSol training will be reviewed tomorrow morning PT by SCEC and SDSC. Let's find a practical way now to merge this and go down the route of symbolic links after the training

@fabian-kutschera
Copy link
Contributor Author

Just added another round of edits for tpv13_qwx, which is good to go from my side as of now.
Next, I'll prepare the refurbished kaikoura notebook for the Gateway.

@fabian-kutschera
Copy link
Contributor Author

Hi @Aangniu , I have just tested all three examples again. Their run times seem fine now using the respective SeisSol app type and were for me:

  • TPV13 (SeisSol app): ~3 min
  • TPV23: (SeisSol nonlinear material app): ~17 min
  • Kaikoura (SeisSol app): ~17 min

From my side, this PR could be merged. @Thomas-Ulrich and @AliceGabriel: the main changes are the jupyter notebooks with the SeisSol output visualizations not relying on vtk/pyvista, whose rendering is currently not supported by the Jupyter Notebook Expanse App from the Gateway.

@Aangniu
Copy link
Contributor

Aangniu commented Jan 21, 2025

@fabian-kutschera: Thank you, Fabian! They look pretty good now. Thanks for the efforts :)

I also think we can merge the current PR now.

I added a link to the ReadMe in the quakeworx sub-directory such that audience without access to Git can directly download all the input files from that link.

@Thomas-Ulrich
Copy link
Contributor

I would propose to add one commit on top of this PR:
see #55
That is using symbolic links for most of the files including binaries, and create copies only when necessary.
If you agree, then feel free to squash my branch to main, instead of this one.
Important to squash and not merge, to get rid of all the large binary file copies that you added.
But if you think I m making too much fuss about 50Mb, just ignore it.
(but adding large unnecessary files to a github repository is bad practice, because that 50 extra Mb that we will always have to clone in the future (maybe github is clever enough to recognize duplicated files though)).

@Aangniu
Copy link
Contributor

Aangniu commented Jan 21, 2025

Thank you for creating a new PR with a better practice!

I would like to confirm if I understand it correctly :) Would you suggestion to squash PR 55 only? Then the audience will get those input files in the quakeworx directory as well, once they pull the repository?

@Thomas-Ulrich
Copy link
Contributor

PR55 contains pr 54 (54 + 1 commit).
you can try if the behaviour is the one you want to test:

git clone --branch thomas/update_pr54 https://github.com/SeisSol/Training

if I copy then

ulrich@ulrich-ThinkPad-T490s:~/trash/test_training/aaaa$ cp -aLR /home/ulrich/trash/test_training/Training/quakeworx/tpv13 .
-a: Archive mode to preserve the directory structure, file attributes, and metadata.
-L: Dereference symbolic links, copying the actual files or directories they point to instead of the links themselves.

Then it works.

@Aangniu
Copy link
Contributor

Aangniu commented Jan 21, 2025

Great! Thanks, Thomas! I just tested. It is working on your branch. I think your method would work for us in this way :)

@fabian-kutschera : I also tested on the Gateway Jupyter Notebook server, we can use:

git clone --branch thomas/update_pr54 https://github.com/SeisSol/Training

and then

cp -aLR Training/quakeworx/ .

The quakeworx/ folder with all the files available should be directly download from the Gateway job outputs. This would serve the purpose for us :)

@fabian-kutschera
Copy link
Contributor Author

Thanks for your help @Thomas-Ulrich and thanks for testing @Aangniu!
I will try to merge #55 now, makes it even easier than cloning a specific branch.

@fabian-kutschera
Copy link
Contributor Author

Closing this PR as #55 has been merged successfully.

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.

4 participants