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

Allow for user modification of the working directory (default of site-packages/covalent_ui is also maybe a bug) #1592

Closed
Andrew-S-Rosen opened this issue Apr 23, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@Andrew-S-Rosen
Copy link
Contributor

Andrew-S-Rosen commented Apr 23, 2023

Note: This is a feature addition, not a docs update

What should we add?

Here are my suggestions, which I motivate further below:

  1. Introduce a configuration option in all executors to set where the base "workdir" is (like the remote_workdir in the SLURM plugin). --> moved to Working directory is not appropriate for local executors #1619.

  2. Perhaps more importantly, introduce a configuration option to let the user decide if they want a given calculation's "workdir" to be done in a unique subfolder (e.g. at workdir/dispatch_id/node_number) both for organizational purposes and to prevent overwriting. --> moved to Allow for the creation of unique subfolders in the current working directory to avoid file overwriting #1628.

Why should we add it?

  1. It is not especially clear from the docs where files generated at runtime are stored (or in other words, what the current working directory is from covalent's perspective at runtime). See the example below.
import covalent as ct
import os

@ct.lattice
@ct.electron
def job(val1, val2):
    with open("job.txt", "w") as w:
        w.write(str(val1 + val2))
    return "Done!"

dispatch_id = ct.dispatch(job)(1, 2)
result = ct.get_result(dispatch_id)
print(result)

In this example, a file job.txt gets written out to the filesystem. It was a bit of an undesirable surprise to me to find out that, in my case, it was being written out to ~/anaconda3/envs/covalent/lib/python3.9/site-packages/covalent_ui. If there's already a way to modify this, it should be mentioned in the docs.

  1. In many quantum chemistry codes, files are often written out to the filesystem at runtime. It's crucial to make sure that these files are not overwriting each other if multiple calculations are going on simultaneously. In the example below, the files are overwriting each other because they are both written out to the ~/anaconda3/envs/covalent/lib/python3.9/site-packages/covalent_ui directory by default, as described before. In more complex examples, this can lead to unexpected errors when electrons are being executed in parallel (and also it's bad to lose calculation files in general). I will note that this is also present with other executors. For instance, with the SLURM plugin, everything is written out to the remote_workdir in a similar way. If there is already an option to do this, it should be mentioned in the docs.
import covalent as ct
import os

@ct.electron
def job(val1, val2):
    with open("job.txt", "w") as w:
        w.write(str(val1 + val2))
    return "Done!"

@ct.lattice
def workflow(val1, val2, val3, val4):
    job1 = job(val1, val2)
    job2 = job(val3, val4)
    return "Done!"

dispatch_id = ct.dispatch(workflow)(1, 2, 3, 4)
result = ct.get_result(dispatch_id)
print(result)
@Andrew-S-Rosen Andrew-S-Rosen added the documentation Improvements or additions to documentation label Apr 23, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Additional details should be provided about where files are stored Additional clarification should be provided about dealing with files generated at runtime Apr 23, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Additional clarification should be provided about dealing with files generated at runtime Additional clarification should be provided about dealing with files generated at runtime since site-packages/covalent_ui is unexpected Apr 23, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Additional clarification should be provided about dealing with files generated at runtime since site-packages/covalent_ui is unexpected Clarification about files generated at runtime since site-packages/covalent_ui is unexpected Apr 23, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Clarification about files generated at runtime since site-packages/covalent_ui is unexpected Clarification about location of files generated at runtime since site-packages/covalent_ui is unexpected Apr 23, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Clarification about location of files generated at runtime since site-packages/covalent_ui is unexpected User modification of the working directory (site-packages/covalent_ui is unexpected) Apr 24, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title User modification of the working directory (site-packages/covalent_ui is unexpected) Feature suggestion + docs update: Allow for user modification of the working directory (and clarify behavior since site-packages/covalent_ui is unexpected) Apr 25, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Feature suggestion + docs update: Allow for user modification of the working directory (and clarify behavior since site-packages/covalent_ui is unexpected) Allow for user modification of the working directory (and clarify behavior since site-packages/covalent_ui is unexpected) Apr 25, 2023
@Andrew-S-Rosen
Copy link
Contributor Author

Andrew-S-Rosen commented May 3, 2023

@santoshkumarradha --- could you switch the label to "feature?" Thanks!

This is a pretty mission-critical issue to me, so I will do my best to work on a PR next week, assuming I've understood the scenario correctly.

@santoshkumarradha
Copy link
Member

hey @arosen93, thanks again for this insightful issue, please keep 'em coming.

Default Working Directory and Executor Dependency

The default working directory for an Electron executed locally is determined by the result_dir configuration option. However, it's important to note that for other remote executors, the working directory is executor-dependent. This means that the location where files are written during execution may vary depending on the executor being used.

Best Practices for File Handling in Covalent

When manually reading or writing files within Covalent, it's always a good practice to use absolute paths. By doing so, you can avoid potential confusion or issues related to file handling.

Here's an example that demonstrates the use of absolute paths in a Covalent workflow:

import covalent as ct
from pathlib import Path

@ct.electron
def job(val1, val2,file):
    with open(file, "w") as w:
        w.write(str(val1 + val2))
    return "Done!"

@ct.lattice
def workflow(input_file, val1, val2, val3, val4):
    job1 = job(val1, val2,input_file)
    return job1

input_file = Path("./example1.txt").absolute()

dispatch_id = ct.dispatch(workflow)(input_file, 1, 2, 3, 4)
result = ct.get_result(dispatch_id, wait=True)
print(result)

Handling Output Files generated inside electron

For chemistry codes that generate output files during calculations, it is crucial to ensure that these files are not overwritten when multiple calculations are executed simultaneously. To achieve this, you can create separate folders for each calculation and use a decorator that changes the working directory to the desired absolute path before executing the function.
Here's an example that demonstrates the use of a decorator to change the working directory and ensure that output files from different calculations do not overwrite each other:

import covalent as ct
from pathlib import Path


import os
from functools import wraps

def change_dir_and_execute(directory):
    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            current_dir = os.getcwd()
            try:
                os.chdir(directory)
                result = func(*args, **kwargs)
            finally:
                os.chdir(current_dir)
            return result
        return wrapper
    return decorator


path=Path(".").absolute()

@ct.electron
@change_dir_and_execute(path)
def job(val1, val2,file):
    with open(file, "w") as w:
        w.write(str(val1 + val2))
    return Path(".").absolute()

@ct.lattice
def workflow(file, val1, val2, val3, val4):
    job1 = job(val1, val2,file)
    return job1


file="example.txt"
dispatch_id = ct.dispatch(workflow)(file, 1, 2, 3, 4)
result = ct.get_result(dispatch_id, wait=True)
print(result)

In this example, the Electron is executed in the specified path (the current directory), ensuring that output files from different calculations are written to separate directories and not overwritten.

I hope this explanation helps address your concerns and suggestions. If you have any specific recommendations on how we can make the documentation clearer or any other concerns, please feel free to share your thoughts. These real-world usage questions are valuable, and we have encountered similar challenges while working on our calculations as well. As this is not a technical issue but rather a pointer on best practices and usage, would it be okay to move this to the discussions section? This way, other users who might face similar questions or concerns can benefit from the information and the proposed solutions.

@Andrew-S-Rosen
Copy link
Contributor Author

Andrew-S-Rosen commented May 4, 2023

Thanks for the reply, @santoshkumarradha! A few comments in return:

Re: The working directory

The default working directory for an Electron executed locally is determined by the result_dir configuration option. However, it's important to note that for other remote executors, the working directory is executor-dependent. This means that the location where files are written during execution may vary depending on the executor being used.

Based on what I'm seeing, I don't think this is entirely true. I did a clean install of covalent 0.220.0 on Ubuntu and ran my sample workflow above but this time specified executor="local" in the Electron just to be sure. The results_dir that is returned is /home/rosen/.local/share/covalent/data. However, the job.txt file is written out to where the covalent package is installed, which for me is at ~/software/miniconda/envs/covalent/lib/python3.9/site-packages/covalent_ui. I believe this is at the very least unexpected behavior but I would argue is a "bug." The same behavior is observed for the Dask executor.

Here is my config just as further support:

{'sdk': {'config_file': '/home/rosen/.config/covalent/covalent.conf',
  'log_dir': '/home/rosen/.cache/covalent',
  'log_level': 'warning',
  'enable_logging': 'true',
  'executor_dir': '/home/rosen/.config/covalent/executor_plugins',
  'no_cluster': 'false',
  'exhaustive_postprocess': 'true'},
 'dispatcher': {'address': 'localhost',
  'port': 48008,
  'cache_dir': '/home/rosen/.cache/covalent',
  'results_dir': '/home/rosen/.local/share/covalent/data',
  'log_dir': '/home/rosen/.cache/covalent',
  'db_path': '/home/rosen/.local/share/covalent/dispatcher_db.sqlite'},
 'dask': {'cache_dir': '/home/rosen/.cache/covalent',
  'log_dir': '/home/rosen/.cache/covalent',
  'mem_per_worker': 'auto',
  'threads_per_worker': 1,
  'num_workers': 20,
  'scheduler_address': 'tcp://127.0.0.1:32913',
  'dashboard_link': 'http://127.0.0.1:8787/status',
  'process_info': "<DaskCluster name='LocalDaskCluster' parent=740 started>",
  'pid': 746,
  'admin_host': '127.0.0.1',
  'admin_port': 49771},
 'workflow_data': {'storage_type': 'local',
  'base_dir': '/home/rosen/.local/share/covalent/workflow_data'},
 'user_interface': {'address': 'localhost',
  'port': 48008,
  'dev_port': 49009,
  'log_dir': '/home/rosen/.cache/covalent'},
 'executors': {'local': {'log_stdout': 'stdout.log',
   'log_stderr': 'stderr.log',
   'cache_dir': '/home/rosen/.cache/covalent'},
  'dask': {'log_stdout': 'stdout.log',
   'log_stderr': 'stderr.log',
   'cache_dir': '/home/rosen/.cache/covalent'},
  'remote_executor': {'poll_freq': 15,
   'remote_cache': '.cache/covalent',
   'credentials_file': ''},
  'slurm': {'username': '',
   'address': '',
   'ssh_key_file': '',
   'remote_workdir': 'covalent-workdir',
   'cache_dir': '/home/rosen/.cache/covalent',
   'poll_freq': 30,
   'cleanup': True,
   'options': {'parsable': ''},
   'srun_options': {}}}}

As a result, it is not currently possible to specify or alter the current working directory. It is fixed to where the Python package for Covalent is, which I don't think is desirable. This should probably be a configuration option.

Re: Best Practices for File Handling in Covalent

When manually reading or writing files within Covalent, it's always a good practice to use absolute paths. By doing so, you can avoid potential confusion or issues related to file handling.

I definitely agree absolute paths are best! That being said, in practice, it is rare that I can control where the files are written out at runtime. That is usually specified by the external code that is being run and is often the current working directory. Otherwise, definitely!

Re: Handling Output Files generated inside electron

To achieve this, you can create separate folders for each calculation and use a decorator that changes the working directory to the desired absolute path before executing the function.

Thank you for sharing this workaround! I appreciate it! I think it is still not fully satisfying. My main reason behind this is that, while it can be done as you suggest (which I will confirm), it is certainly not concise or clean. One of the major benefits of Covalent as a whole is that there is minimal friction to go from writing a function to running a complex workflow. Given the many foreseeable use cases (at least in my field) where significant I/O is written out to the current working directory at runtime (without this being something that can be changed), always needing this somewhat verbose approach is a bit less than ideal. I would still push for a configuration variable of sorts that the user could specify that would write out all files to something like /dispatch_id/node_number in the current working directory or the results_dir rather than all in one single place where overwriting becomes a concern.

As this is not a technical issue but rather a pointer on best practices and usage, would it be okay to move this to the discussions section? This way, other users who might face similar questions or concerns can benefit from the information and the proposed solutions.

I think parts of this can certainly be migrated over, but I still think that an issue should remain given the points above. Happy to discuss further if you think otherwise!

Thanks a ton for your answer, and I look forward to your reply! I'll continue to think about the best way to potentially address this.

@Andrew-S-Rosen Andrew-S-Rosen changed the title Allow for user modification of the working directory (and clarify behavior since site-packages/covalent_ui is unexpected) Allow for user modification of the working directory (default of site-packages/covalent_ui is also maybe a bug) May 4, 2023
@santoshkumarradha
Copy link
Member

Oops, Let me get back on the first part (I may be wrong about it with the recent changes, will quickly circle with the team)

But meanwhile I agree with this,

Thank you for sharing this workaround! I appreciate it! I think it is still not fully satisfying. My main reason behind this is that, while it can be done as you suggest (which I will confirm), it is certainly not concise or clean. One of the major benefits of Covalent as a whole is that there is minimal friction to go from writing a function to running a complex workflow. Given the many foreseeable use cases (at least in my field) where significant I/O is written out to the current working directory at runtime (without this being something that can be changed), always needing this somewhat verbose approach is a bit less than ideal. I would still push for a configuration variable of sorts that the user could specify that would write out all files to something like /dispatch_id/node_number in the current working directory or the results_dir rather than all in one single place where overwriting becomes a concern.

Could you let me know an ideal UX you would prefer for changing the "working" directory of the electron? (something that is consistent for both local and remote executors) Is the UX that the default execution folder is indeed a folder that is result_dir/dispatch_id/node_number?

One thing that we have not worked a lot on docs is also file transfer mechanisms and adding call backs/ call befores which can run pre run and post run hooks for data transfers as well, would you think these are something that would work as well?

@Andrew-S-Rosen Andrew-S-Rosen changed the title Allow for user modification of the working directory (default of site-packages/covalent_ui is also maybe a bug) Allow for the creation of unique working subdirectories in a way that can avoid file overwriting May 5, 2023
@Andrew-S-Rosen
Copy link
Contributor Author

Andrew-S-Rosen commented May 5, 2023

Oops, Let me get back on the first part (I may be wrong about it with the recent changes, will quickly circle with the team)

Thanks! To make this more manageable, I have opened a new issue at #1619 for this part of my original post. The remainder of the current issue can be dedicated to the feature for creating subdirectories that would avoid file overwriting.

Could you let me know an ideal UX you would prefer for changing the "working" directory of the electron? (something that is consistent for both local and remote executors) Is the UX that the default execution folder is indeed a folder that is result_dir/dispatch_id/node_number?

I think my general recommendation is that there should be a keyword argument of the type create_folders: bool. If set to True, then wherever the default working directory is (which may depend on the executor), Covalent would automatically create subfolders of the form dispatch_id/node_number in the current working directory. Each calculation node would cd into its respective dispatch_id/node_number directory to avoid file overwriting concerns by any external program writing to the current working directory during runtime.

Note that the base directory for this may be distinct from the results_dir because the results_dir will typically be where the dispatcher is being called from, and we don't necessarily want to transport files generated at runtime to go there (that should be something the user specifically requests via the file transfer mechanisms you suggested). For instance, in the SLURM executor, the current working directory is set by the remote_workdir and not the results_dir, which makes sense.

One thing that we have not worked a lot on docs is also file transfer mechanisms and adding call backs/ call befores which can run pre run and post run hooks for data transfers as well, would you think these are something that would work as well?

Unfortunately, this route isn't sufficient for the problem. We want to make sure that if there are multiple ongoing calculations that write files out to the current working directory during runtime that they do not overwrite one another. Many computational chemistry codes are hard-coded to rely on being able to write and read input and output files in the current working directory throughout the calculation itself. Transferring files before or after the calculation finishes is a separate issue that should be left to the user, as you suggested.

I moved this to #1628 to prevent confusion and closed this issue.

@Andrew-S-Rosen Andrew-S-Rosen changed the title Allow for the creation of unique working subdirectories in a way that can avoid file overwriting Allow for the creation of unique subfolders in the current working directory to avoid file overwriting May 5, 2023
@Andrew-S-Rosen Andrew-S-Rosen changed the title Allow for the creation of unique subfolders in the current working directory to avoid file overwriting Allow for user modification of the working directory (default of site-packages/covalent_ui is also maybe a bug) May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants