Skip to content

Conversation

@MarkKoz
Copy link
Contributor

@MarkKoz MarkKoz commented Nov 6, 2023

Various fixes for issues I encountered when trying to test out DirectML (which still does not work by the way). See the commit messages for more information. Happy to split this up into multiple PRs if that is easier to review.

By far the biggest issue with development still is the installation of Python dependencies. The provided dependencies are out of date compared to the latest xvasynth release, and even then, a couple of dependencies (h2p_parser and pkuseg) have broken setups that require manual fixes.

I have managed to circumvent these issues locally. However, I am hoping there is a simpler solution that I missed, so I'm holding off on adding instructions for the relatively complex set of steps to get all the dependencies working locally. @DanRuta is there a more up to date set of requirements you can upload to the repo?

This was causing the logged stack to be undefined.
When style_embeddings is imported, it will load style embeddings from a
directory. It was assuming this directory already exists, but it will
not when running the checked-out repository. This will cause an error
to bubble up to script.js which will break the app very badly since a
lot of critical code will never run.
This tool is used by the package-* scripts in package.json.
Make the new requirements files actually installable with pip.
Previously, their contents were just the outputs of `pip list`.
Very rarely do we truly need to capture all exceptions. It will prevent
things like keyboard interrupts and system exits from propagating.
Having a broad except clause also leads to confusing tracebacks since
if there is an actual error, we will try the next set of imports anyway.

What we really want is to keep trying different import paths until we
find the right one. Thus, we only need to catch ModuleNotFoundError.
@DanRuta
Copy link
Owner

DanRuta commented Nov 10, 2023

Thank you so much for these changes! Actually, the local deps are up-to-date. I am indeed aware of the manual fixes required for those dependencies. I haven't yet spent the time to properly document them, as I was hoping I'd find a way to not have to do the manual fixes, but I haven't yet had the time to explore a proper fix yet. Are you in the Discord server, btw?

@DanRuta DanRuta merged commit e9793a2 into DanRuta:master Nov 10, 2023
@MarkKoz
Copy link
Contributor Author

MarkKoz commented Nov 11, 2023

You're welcome! I'm not in the Discord server but don't mind joining.

I compared the dependencies bundled with the latest release against my virtual environment when I installed from the requirements file. I recall that either some dependencies were missing or out of date.

For h2p_parser, I have a PR here to fix it ionite34/h2p-parser#42. For the time being we could point the requirements file to install from my fork instead.

For pkuseg, I followed explosion/spaCy#7370 (comment) to get a local build working. That comment mentions a fork of the package https://pypi.org/project/spacy-pkuseg/ which we can try using instead of the original. The fork fortunately has wheels for Python 3.9.

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