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

Fix notebook: Colab now has python 3.8, fix imports, mitigate UTF-8 glitch #285

Merged
merged 4 commits into from
Apr 10, 2023

Conversation

vaclavhanzl
Copy link
Contributor

@vaclavhanzl vaclavhanzl commented Mar 5, 2023

Make the colab notebook work (fix issue #286):

  • Python 3.8 is now what we get in Colab, use it in our Conda as well
  • Fix imports, make some unconditional - chain of depencencies makes them needed even for relax_prediction==False (overwrite_b_factors() comes from file which imports openmm)
  • Mitigate spurious error "A UTF-8 locale is required. Got ANSI_X3.4-1968" which makes all shell commands defunct - use shutil.make_archive()
  • In the second commit, mitigate the ANSI_X3.4-1968 problem yet much better, making %shell work again

The strange "ANSI_X3.4-1968" thing happens sometimes not only to us. When it happens, no shell commands work anymore. Likely OpenMM is the culprit. I used the same mitigation as AlphaFold did for the same problem (use shutil, see their patch), this way our and their notebooks do not differ more than necessary. But I found this solution insuficient because it just avoids using %shell, leaving it broken. So I added the second commit which I think is superior to AlphaFold's fix (which can still stay there).

…litch

* Python 3.8 is now what we get in Colab, use it in our Conda as well
* Fix imports, make some unconditional - chain of depencencies makes them
  needed even for relax_prediction==False
* Mitigate spurious error "A UTF-8 locale is required. Got ANSI_X3.4-1968"
  which makes all shell commands defunct - use shutil.make_archive()
@vaclavhanzl
Copy link
Contributor Author

Colab is easily broken by openmm and other software messing with locale.
This mitigation patch lets the %shell and ! work again.
It is much stronger solution than just avoiding shell when making zip.
Previously protein got computed and shown in both cases but only
saved to the downloaded zip when relaxation was done.
@vaclavhanzl
Copy link
Contributor Author

vaclavhanzl commented Mar 6, 2023

I noticed that with relaxation off, protein is computed and shown but NOT saved to the downloaded zip. My third commit should fix this. I just tested in Colab that the protein is now saved with relaxation off. Did not yet manage to re-test with relaxation on.

@vaclavhanzl
Copy link
Contributor Author

vaclavhanzl commented Mar 6, 2023

Question (@gahdritz ?): What was the idea behind conditional installation/import based on relax_prediction? Save time? Make it more robust? As I had to install openmm unconditionally (to get overwrite_b_factors from file depending on openmm), I guess I can as well install/import the rest unconditionally, too? Maybe the - now avoided - UTF-8 glitch was the reason to avoid openmm? Or did you intend to further refactor codebase to make openmm avoidable?

I did another commit removing install/import dependencies on relax_prediction, now one can easily repeat just the first and last cells to get protein with relax and without. For me this is the final version, unless somebody requests changes.

This way user can repeat just the first and last cell to get both versions.
@gahdritz gahdritz merged commit 685e8b5 into aqlaboratory:main Apr 10, 2023
@gahdritz
Copy link
Collaborator

Yeah it was initially intended to save time. Thanks for the PR!

@vaclavhanzl vaclavhanzl mentioned this pull request Apr 10, 2023
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.

2 participants