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

Add option to clean remote folder when deleting nodes #6732

Open
superstar54 opened this issue Jan 27, 2025 · 15 comments · May be fixed by #6756
Open

Add option to clean remote folder when deleting nodes #6732

superstar54 opened this issue Jan 27, 2025 · 15 comments · May be fixed by #6756
Labels
type/feature request status undecided

Comments

@superstar54
Copy link
Member

I want to clean the remote folder when I delete a node. For the moment, I need to find all the call jobs that are called by this node, and clean the workdir, and then delete the node.

It would be useful to add an option to clean the remote folders when deleting the node.

e.g., verdi node delete --clear-workdir <pk>

@superstar54 superstar54 added the type/feature request status undecided label Jan 27, 2025
@khsrali
Copy link
Contributor

khsrali commented Jan 28, 2025

Somehow related: #4693
would be nice while providing this feature, one would also address that issue.

@Kaustbh
Copy link

Kaustbh commented Feb 11, 2025

Hi @superstar54 , I’m interested in solving this issue. Could you guide me on how to get started? For example, are there any specific steps I should follow, or resources I should review to better understand the context of this issue?

@superstar54
Copy link
Member Author

Hi @Kaustbh , thanks for the interest. You may need to wait for the maintainers to decide.

@khsrali
Copy link
Contributor

khsrali commented Feb 12, 2025

I'm already working on this issue, soon will open a PR on it.

@unkcpz
Copy link
Member

unkcpz commented Feb 15, 2025

I want to clean the remote folder when I delete a node. For the moment, I need to find all the call jobs that are called by this node

At the moment, in aiida-core, only WorkChain will be "called by this node". If it is only workgraph related, since it is not integrated to aiida-core, please leave the features there only at the moment.

In aiida-core, at the moment, only calcjob and workchain will have remote resource that need to cleaned so I think it makes no sense to add functionality to add --clean-remote-folder to all node type since there are tons of other node type that has no remote folder attached. Even if you decide to add this to aiida-core. will it make more sense to add to verdi process subcommand?

@superstar54
Copy link
Member Author

At the moment, in aiida-core, only WorkChain will be "called by this node". If it is only workgraph related, since it is not integrated to aiida-core, please leave the features there only at the moment.

Nothing special to WorkGraph. From a user's point of view, an option to clean the remote folder when deleting the node is very useful. Note when you delete a calcfunction node, it could also delete the associated WorkChain/CalcJob, thus the --clear-workdir can also be applied.

will it make more sense to add to verdi process subcommand?

Could you explain why it makes more sense to add to verdi process instead of verdi node?

@unkcpz
Copy link
Member

unkcpz commented Feb 19, 2025

Note when you delete a calcfunction node, it could also delete the associated WorkChain/CalcJob, thus the --clear-workdir can also be applied.

Where you see the calls of workchain and calcjob from a calcfunction?

I want to clean the remote folder when I delete a node. For the moment, I need to find all the call jobs that are called by this node, and clean the workdir, and then delete the node.

Besides WorkChain and WorkGraph, I really don't see any other cases you were mentioning. What is your real example? What is the node you were deleted?

From a user's point of view, an option to clean the remote folder when deleting the node is very useful.

The option exist, and that is verdi calcjob cleanworkdir. Now the option you asked is a batch operation of "search the decent calcjobs and then call verdi calcjob cleanworkdir". I against it because it has strong side effect.

Could you explain why it makes more sense to add to verdi process instead of verdi node?

Because only "processes" will have remote workdir attached.

@unkcpz
Copy link
Member

unkcpz commented Feb 19, 2025

I may have a better solution. From my comment above: "Now the option you asked is a batch operation of "search the descendants calcjobs and then call verdi calcjob cleanworkdir". "

It can actually provide a command that do "search the decent calcjobs" and we find a way to pipe the verdi commands. Then you can do batch operations using the stdout from previous command.

Worst case, have two command separately implemented and having a command using click to call them in sequence.

@superstar54
Copy link
Member Author

superstar54 commented Feb 20, 2025

Where you see the calls of workchain and calcjob from a calcfunction?
Besides WorkChain and WorkGraph, I really don't see any other cases you were mentioning. What is your real example? What is the node you were deleted?

Because of the traversal_rule, any AiiDA node (including the CalcFunctionNode) could have an associated calcjob and remote folder.

As a user, when I delete a node, I want to have an option to delete the associated data, both locally and remotely.

Because only "processes" will have remote workdir attached.

verdi process command is designed to interact with Process (most of the time, the living Process), not the Node. Process does not have remote workdir attached, but its Node has the workdir attached.

It can actually provide a command that do "search the decent calcjobs" and we find a way to pipe the verdi commands. Then you can do batch operations using the stdout from previous command.

I don't think it's a good solution, because each verdi calcjob cleanworkdir will open a connection to the remote computer, which we want to avoid.

@khsrali
Copy link
Contributor

khsrali commented Feb 20, 2025

@unkcpz The problem is a process does not have a workdir. And even we take the meaning vaguely that bring about more issues, such as a process first has to stop or killed, etc..

While workdir are properties of nodes not processes, if anything general I'd go with verdi node cleanworkdir

@unkcpz
Copy link
Member

unkcpz commented Feb 20, 2025

I don't think it's a good solution, because each verdi calcjob cleanworkdir will open a connection to the remote computer, which we want to avoid.

It is not there by default in @khsrali's PR it is done by clean_mapping_remote_paths(path_mapping) which try to curate the remote computers first. This part of code is copied from verdi calcjob cleanworkdir. So for verdi calcjob cleanworkdir, it should be able to accept multiple pks and do clean with single transport connection.

Because of the traversal_rule, any AiiDA node (including the CalcFunctionNode) could have an associated calcjob and remote folder.

If you mean to clean the workdir of RemoteData that is the output of CalcFunctionNode, it is not in Ali's implementation as well, since he filter out that will only clean for CalcJobNode.

While workdir are properties of nodes not processes

@khsrali, only RemoteData node has workdir, but not all nodes. You can do it in you PR for "general nodes" because you use the GraphTraversalRule.DELETE for verdi node delete. My whole point here is you do two things in one command, delete nodes and clear up the remote workdir while you put the operation in node delete. It has side effect that clean a code node can also trigger the clear up all the remote data along the graph. In the description of your PR, you say " one can also remove all remote directories of workflows and workchains." which is conflict with the effect here. Maybe what @superstar54 try to ask was also include the side effect (which i think it can be a good side effect, since when node deleted, it relatively useless to keep the remote data)?

@khsrali
Copy link
Contributor

khsrali commented Feb 20, 2025

@unkcpz

If you mean to clean the workdir of RemoteData that is the output of CalcFunctionNode, it is not in Ali's implementation as >well, since he filter out that will only clean for CalcJobNode.

RemoteData are also getting deleted based on traversal rules.

My whole point here is you do two things in one command, delete nodes and clear up the remote workdir while you put the >operation in node delete

I understand your point. That's why I suggest verdi node cleanworkdir that would only clean directories.

only RemoteData node has workdir, but not all nodes.

That's not true, calcjobs also have working directory.

This thread is getting too long, hard to follow, also is deviating from the main points that we wanted to address.
Let's discuss this today during AiiDA meeting. So to converge and agree on a solution.

@unkcpz
Copy link
Member

unkcpz commented Feb 20, 2025

I understand your point. That's why I suggest verdi node cleanworkdir that would only clean directories.

I think this would be a solution.

Let's discuss this today during AiiDA meeting. So to converge and agree on a solution.

Sure.

@khsrali
Copy link
Contributor

khsrali commented Feb 20, 2025

Update:

We just discussed in aiida meeting. We all agreed verdi node delete --clean-workdir makes sense as it's own, of course the PR #6756 can be further reviewed :)
It might make sense to also introduce verdi node cleanworkdir but that will remain out of the scope for PR #6756

@unkcpz
Copy link
Member

unkcpz commented Feb 20, 2025

Another update:

Pipeline commands can be a solution, the verdi calcjob clean_workdir can accept multiple PKs and open minimal amount of transport to do the operation. If there is a sub-command that verdi node (@khsrali mention verdi node list with proper filter should support it) can support to traverse and get all the calcjobs need to delete, it can then pipeline to verdi calcjob clean_workdir. The issue then is how to well document it to let user know such tricks exist.

It might make sense to also introduce verdi node cleanworkdir but that will remain out of the scope for PR #6756

Yes, it is out the scope of such discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature request status undecided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants