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

Windows support in OpenFL tutorials #549

Open
Maxime-Perret opened this issue Oct 27, 2022 Discussed in #546 · 2 comments
Open

Windows support in OpenFL tutorials #549

Maxime-Perret opened this issue Oct 27, 2022 Discussed in #546 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@Maxime-Perret
Copy link

Maxime-Perret commented Oct 27, 2022

Hello,

I've posted this below as discussion but since I've just updated my post, I figured it would fit as an issue now related to supporting Windows.

Discussed in #546

Originally posted by Maxime-Perret October 26, 2022
Hello,

I've been experimenting with OpenFL trying to run different tutorials. Since supporting Windows was mentioned in #525, I am personally using Windows 11 Pro and ran into two problems which I believe are OS related in the different tutorials.

  • For DataLoader, having num_workers any larger than 0 causes an issue as such. Thus, for Windows users, having num_workers = 0 would be necessary. As far as I know, out of the interactive api folder, this concerns the tutorials Dogs vs Cats, Histology, Histology FedCurv, Kvasir UNet, Lightning MNIST GAN, MVTec PatchSVDD, Market Re-ID, MedMNIST 2D, MedMNIST 3D.

  • Some of the arguments in the plans of the tutorials (in workspace/plan/plan.yaml) contain !!, like for example criterion: !!python/object:torch.nn.modules.loss.CrossEntropyLoss. When parsing the plan in federated/plan/plan.py, line 42 uses safe_load which together causes an issue could not determine a constructor for the tag. Using the fix suggested here, I got it to work by replacing safe_load(yaml_path.read_text()) by load(yaml_path.read_text(), Loader=yaml.Loader).

  • Using the fx pki install command downloads the binaries for step and step_ca. The code to download those filters the available assets on github by content_type to be applications/gzip in openfl/component/ca/ca.py, l.74. However, while the Linux assets are .tar.gz, the Windows assets are .zip, with content_type then being application/zip. This restriction makes it impossible to find assets for Windows. I tried forcing it to accept but it still fails later on regardless.

Best

Edit: Updated my post after fixing some of my problems with a clean reinstall of Python+Anaconda

@Maxime-Perret Maxime-Perret changed the title Windows support and some bugs in OpenFL tutorials Windows support in OpenFL tutorials Oct 27, 2022
@Einse57 Einse57 added the enhancement New feature or request label Oct 27, 2022
@psfoley psfoley assigned itrushkin and unassigned psfoley Jan 18, 2023
@itrushkin
Copy link
Contributor

itrushkin commented Jan 24, 2023

Hi @Maxime-Perret, thank you for providing feedback regarding the experience using OpenFL.

This will be fixed in #708.

I have run PyTorch_MedMNIST2D tutorial, which uses the criterion in additional train task keyword arguments. There is no criterion section in plan.yaml. Could you please elaborate more on the use case?
Our project requires using safe_load YAML file reading due to the risk involved in loading a document from untrusted input.

  • Using the fx pki install command downloads the binaries for step and step_ca. The code to download those filters the available assets on github by content_type to be applications/gzip in openfl/component/ca/ca.py, l.74. However, while the Linux assets are .tar.gz, the Windows assets are .zip, with content_type then being application/zip. This restriction makes it impossible to find assets for Windows. I tried forcing it to accept but it still fails later on regardless.

Downloading of the step-ca was changed. I personally have tested it on both Linux and Windows systems. See the code.

@itrushkin
Copy link
Contributor

Because of having no response in 2 weeks, taking the issue as stale. Related PR will be merged soon. If there are any additions/comments please feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants