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

Improvements on filemovements #1162

Merged
merged 15 commits into from May 10, 2024
Merged

Conversation

mandresm
Copy link
Contributor

@mandresm mandresm commented Apr 19, 2024

This PR aims to:

  • Remove the intermediate copying by default (except for config, input, and forcing. The defaults can be overridden by general.intermediate_movement = [list of filetypes that go to the intermediate folders]
  • [to be done in another PR] Parallelize the file movements (WIP)
  • Profiling

This is to reduce the copying times of @tsemmler05, @a270067 and @mjy1011.

@JanStreffing and @mjy1011, this branch is not in it's final stage, but it would be a great help if you can check that is working for an actual simulation. I've tested already a bit but not in depth.

… no files are copied to the intermediate folders
@JanStreffing
Copy link
Contributor

This week I will be back. Hopefully be Wednesday or so, I get to give this a test.

@mandresm mandresm requested a review from pgierz April 29, 2024 13:08
@mandresm
Copy link
Contributor Author

I consider this finished, so feel free to give it a try. I will work on the parallelization on another PR, and that would take longer.

PD: Wednesday is bank holiday ;)

@mjy1011
Copy link
Collaborator

mjy1011 commented May 2, 2024

Hi Miguel, thank you very much for the modification.
On Aleph when we restart experiment with the updated codes, attached error occurs.
It seems oifs restart info files (rcf, waminfo)' path are not detected correctly.
run_error

Do we need to modify any other files? for example, /esm_tools/configs/esm_software/esm_runscripts/defaults.yaml ?

@mandresm
Copy link
Contributor Author

mandresm commented May 2, 2024

Hi @mjy1011, I'll be looking into this today. The problem is that the preprocessing scripts expect files in the intermediate folders.

@mandresm
Copy link
Contributor Author

mandresm commented May 2, 2024

Hi @mjy1011,

I've got done some progress today with the issue, but there are some weird errors still. I think I am close, but I'll have to continue tomorrow.

@mjy1011
Copy link
Collaborator

mjy1011 commented May 2, 2024

Hi @mjy1011,

I've got done some progress today with the issue, but there are some weird errors still. I think I am close, but I'll have to continue tomorrow.

That's nice. Thank you for the update.

@mandresm mandresm force-pushed the feat/file_movement_improvements branch from 12edb36 to 339da3d Compare May 3, 2024 10:58
…writing files in the general experiment folder when copying directly from work
@mandresm
Copy link
Contributor Author

mandresm commented May 4, 2024

Hi @mjy1011,

That was a real tough one... but I think it's working now. Can you please try?

@JanStreffing, can you try if the results are bit identical with release and this branch? I am getting non-bit identical results in some OpenIFS output files and restarts, and I am wondering if that's normal? (for example, maybe the results in the files are identical but there is something different in the headers)

@mjy1011
Copy link
Collaborator

mjy1011 commented May 5, 2024

Hi @mjy1011,

That was a real tough one... but I think it's working now. Can you please try?

@JanStreffing, can you try if the results are bit identical with release and this branch? I am getting non-bit identical results in some OpenIFS output files and restarts, and I am wondering if that's normal? (for example, maybe the results in the files are identical but there is something different in the headers)

Hi ~~
I am waiting for the pre-running job to be finished. I'll try and let you know in a few days. Nice work again!

Copy link
Member

@pgierz pgierz left a comment

Choose a reason for hiding this comment

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

Looks good!

Only a few minor changes (and one typo 😉)

src/esm_runscripts/filelists.py Outdated Show resolved Hide resolved
src/esm_runscripts/filelists.py Show resolved Hide resolved
src/esm_runscripts/filelists.py Show resolved Hide resolved
src/esm_runscripts/filelists.py Outdated Show resolved Hide resolved
src/esm_runscripts/filelists.py Outdated Show resolved Hide resolved
src/esm_runscripts/filelists.py Show resolved Hide resolved
mandresm and others added 4 commits May 6, 2024 15:31
Co-authored-by: Paul Gierz <pgierz@awi.de>
Co-authored-by: Paul Gierz <pgierz@awi.de>
Co-authored-by: Paul Gierz <pgierz@awi.de>
Co-authored-by: Paul Gierz <pgierz@awi.de>
@mandresm
Copy link
Contributor Author

mandresm commented May 6, 2024

Looks good!

Only a few minor changes (and one typo 😉)

Thanks for the review!

Copy link
Member

@pgierz pgierz left a comment

Choose a reason for hiding this comment

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

I agree with the error approach!

src/esm_runscripts/filelists.py Show resolved Hide resolved
src/esm_runscripts/filelists.py Outdated Show resolved Hide resolved
@mjy1011
Copy link
Collaborator

mjy1011 commented May 10, 2024

Hi @mjy1011,
That was a real tough one... but I think it's working now. Can you please try?
@JanStreffing, can you try if the results are bit identical with release and this branch? I am getting non-bit identical results in some OpenIFS output files and restarts, and I am wondering if that's normal? (for example, maybe the results in the files are identical but there is something different in the headers)

Hi ~~ I am waiting for the pre-running job to be finished. I'll try and let you know in a few days. Nice work again!

Just confirmed it is working right! And most of all, saving more than 2 hours, amazing. Thank you very much!

@mandresm
Copy link
Contributor Author

#approve-changes

@mandresm
Copy link
Contributor Author

Just confirmed it is working right! And most of all, saving more than 2 hours, amazing. Thank you very much!

You are welcome, and thanks for your patience with this issue :)

Co-authored-by: Paul Gierz <pgierz@awi.de>
@mandresm
Copy link
Contributor Author

#bump

@mandresm mandresm merged commit 246dacd into release May 10, 2024
@mandresm mandresm deleted the feat/file_movement_improvements branch May 10, 2024 10:54
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.

None yet

4 participants