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

Support forward files for init_surf #909

Merged

Conversation

HuangJiameng
Copy link
Collaborator

Support user use "forward_files" in machine parameters to upload other files besides POSCAR, INCAR, POTCAR. (forwarded files for each task)

@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2022

Codecov Report

Merging #909 (0065cdb) into devel (fdc0622) will increase coverage by 0.08%.
The diff coverage is 12.50%.

@@            Coverage Diff             @@
##            devel     #909      +/-   ##
==========================================
+ Coverage   38.14%   38.23%   +0.08%     
==========================================
  Files          99       99              
  Lines       17766    17803      +37     
==========================================
+ Hits         6777     6807      +30     
- Misses      10989    10996       +7     
Impacted Files Coverage Δ
dpgen/data/surf.py 66.46% <12.50%> (-0.87%) ⬇️
dpgen/generator/arginfo.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@njzjz
Copy link
Member

njzjz commented Aug 23, 2022

Isn't it user_forward_files?

@HuangJiameng
Copy link
Collaborator Author

It is forward_files in machine parameters. https://docs.deepmodeling.com/projects/dpdispatcher/en/latest/task.html#argument:task/forward_files
Am I understanding it wrong?

@njzjz
Copy link
Member

njzjz commented Aug 23, 2022

I think it's this feature:

user_forward_files = mdata.get("train" + "_user_forward_files", [])
forward_files += [os.path.basename(file) for file in user_forward_files]
backward_files += mdata.get("train" + "_user_backward_files", [])

Indeed we should not maintain it everywhere... Create a method instead.

@HuangJiameng
Copy link
Collaborator Author

I think it's this feature:

user_forward_files = mdata.get("train" + "_user_forward_files", [])
forward_files += [os.path.basename(file) for file in user_forward_files]
backward_files += mdata.get("train" + "_user_backward_files", [])

Indeed we should not maintain it everywhere... Create a method instead.

In contrast, I think it is more reasonable to put symlinks in the task path for paths in "forward_files". Otherwise the user should pile everything in the main directory. Besides, should we keep the same parameter name "forward_files" with dpdispatcher?

@njzjz
Copy link
Member

njzjz commented Aug 23, 2022

I think it's this feature:

user_forward_files = mdata.get("train" + "_user_forward_files", [])
forward_files += [os.path.basename(file) for file in user_forward_files]
backward_files += mdata.get("train" + "_user_backward_files", [])

Indeed we should not maintain it everywhere... Create a method instead.

In contrast, I think it is more reasonable to put symlinks in the task path for paths in "forward_files". Otherwise the user should pile everything in the main directory. Besides, should we keep the same parameter name "forward_files" with dpdispatcher?

I don't understand what you mean. The old codes also make symbolic links.

Making an alias is fine, but the old name should be kept for compatibility.

Comment on lines +552 to +555
work_path_list = glob.glob(os.path.join(work_dir, "surf-*"))
task_format = {"fp" : "sys-*"}
for work_path in work_path_list :
symlink_user_forward_files(mdata=mdata, task_type="fp", work_path=work_path, task_format=task_format)
Copy link
Member

@njzjz njzjz Aug 28, 2022

Choose a reason for hiding this comment

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

Why not directly set the format as surf-*/sys-*

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because surf-* seems to align with the meaning of work_path.

@wanghan-iapcm wanghan-iapcm merged commit 5ff7085 into deepmodeling:devel Aug 30, 2022
@HuangJiameng HuangJiameng deleted the fix_init_surf_forward_files branch September 26, 2022 08:22
wanghan-iapcm pushed a commit that referenced this pull request Nov 9, 2022
Since "forward_files" for init_surf is supported (see #909), here add
the relevant doc.
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