jacksund / simmate

The Simulated Materials Ecosystem (Simmate) is a toolbox and framework for computational materials research.
https://simmate.org
BSD 3-Clause "New" or "Revised" License
30 stars 8 forks source link

new simmate release(s) #546

Closed jacksund closed 7 months ago

jacksund commented 8 months ago

@SWeav02 I just saw your repo's new README saying to do a dev install of simmate. Not ideal for new users 😅

@scott-materials and @SWeav02 -- if you two are willing to help "test" the dev versions, we can make new Simmate releases whenever you'd like. The release itself is very quick (~5-10min of work), so I wouldn't mind a weekly release.

The only reason my releases are so spread out is because (1) it takes me a long time to run the full test suite on my laptop and (2) if I make a new release with minor bugs, then I have to run the test suite all over again. The test suite is literally just running all the workflows with small structures and checking that they work, and I currently use this script here. It's just a bunch of VASP workflows, which can take up to 24hrs to run on my 4 cores 😭

... however, if you two are willing to run the test suite on your beefed up computers, I can make a release whenever. This should be easy now that you two know how to do a dev install. I will just a need a "the workflows have been tested and everything looks good" (just open a new issue)-- then I'll make the release for you.

I know @becca9835 has asked for a new release recently, and it's just waiting on a test suite run. Let me know if any of you are willing to help! 😄

SWeav02 commented 8 months ago

I would be very happy to do this. Honestly the main thing that kept me from suggesting adding the new code to a release is that I haven't written a good test script for the updated BadELF code. But I should be able to just add the warrenapp s3 workflows and a new section for badelf workflows. I'll work on updating the script and running it today.

There are also a couple of things that overlap between the main fork of simmate and my own, mostly related to the old badelf workflows. I think I'll probably remove those since they work a bit differently than the updated version and also store info in a different database table which would be confusing for users.

jacksund commented 8 months ago

yeah, feel free to open a PR and add your stuff. Then test your workflows like I did in this script. If you keep everything contained within your app and maybe add some user warnings (as logging messages or adding to your section of the docs), then you are good to go.

I can make a new release before you add your app too -- so this post is more about just running the test suite for me 😃

SWeav02 commented 8 months ago

Ok I've opened a PR draft. I'm making a new env on WarWulf to run the test suite so that I can use a whole node and keep using my desktop . There are already some user warnings in the code, but I'm planning on making some more in depth docs this week.

SWeav02 commented 8 months ago

Ok while I'm getting set up for running the test script, pytest is throwing some errors that look like this:

FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__Pbe] - assert 429 == 200
FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__PbeMetal] - assert 429 == 200
FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__Pbesol] - assert 429 == 200
FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__PrebadelfHse] - assert 429 == 200
FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__PrebadelfPbesol] - assert 429 == 200
FAILED src/simmate/website/workflows/test/test_workflows_views.py::test_workflow_detail_view[StaticEnergy__Warren__Scan] - assert 429 == 200

Based on where the errors are being thrown, I assume this is a Django views issue, but I'm not sure what the best way to fix it would be. Any thoughts @jacksund?

In the meantime, I'm assuming this won't interfere with the test script running workflows so I'm going to go ahead and get that started.

jacksund commented 8 months ago

thanks for doing that! Let me know when you're ready for a review of that PR

as for the failed tests you're seeing, I'll respond over in your PR thread (and you post any other questions over there too!)

SWeav02 commented 8 months ago

One more question (this one is related to the test script so I'm leaving it here :grin:). I ran the test script by submitting a slurm job with a line like this: python path/to/test/script/test_all_workflow_runs.py. I left it without checking much and it looks like its no longer running about 3hr later which means to me it either finished or I ran it incorrectly. The output of the slurm run is just this:

2023-12-19 15:04:07 INFO     Configuring database and workflows 🛠
2023-12-19 20:04:14 INFO     Configuration complete 🚀

I assume if it had failed it would have given me some sort of output at the assert missing_failed_flows == [] line, but I also had assumed that if it would give me some output while it was running the workflows. Does this mean it finished fine or I'm missing something simple?

jacksund commented 8 months ago

python path/to/test/script/test_all_workflow_runs.py

This isn't the command to run the script as that python file only defines the test 😅. You need to run it using pytest and also enable logging to help. I'll write a quick guide for this script tomorrow -- and maybe add a PR so that it's easier to run for you

SWeav02 commented 8 months ago

I'm working on running the test suite and got the the point where I reset the database and use the prebuilt. When I run simmate database update I get this error:

InconsistentMigrationHistory: Migration workflows.0001_initial is applied before its dependency core_components.0002_alter_fingerprint_id_alter_fingerprintpool_id on database 'default'.

I'm assuming this means I'll need to rebuild the prebuilt database. I'll start the process as described here. Since I know downloading the databases takes quite a while, I'll come back to this tomorrow to run the tests.

jacksund commented 8 months ago

yep spot on! the prebuilt is still pointing to the old database & migrations (i.e. I was using django incorrectly for everything <= v0.14.0). The new prebuilt is actually being made right now -- its been running for the last 5 hrs or so. I would wait until I merge #548 before going through it. I'll ping you when its ready 👍

jacksund commented 8 months ago

the database prebuild is still running, but I've uploaded a temporary empty "prebuild" and then merged the PR. You can give this guide a try now: https://jacksund.github.io/simmate/contributing/maintainer_notes/#the-full-test-suite

Just do this for the official main branch. We will handle adding the warren_lab and badelf apps to the test suite as a 2nd step (and potentially a 2nd release).

SWeav02 commented 8 months ago

Sounds good! I'm going through the test suite guide you sent now. Thanks for doing that :smile:

jacksund commented 8 months ago

I just uploaded the new prebuilt database for you to try. If you already downloaded the temporary one, just delete the folder at ~/simmate/sqlite-prebuilds and then rerun the database reset command -- that will grab the new db I just added. It should be a 1.5gb download and then unpacks to ~9gb now.

Let me know if there are any issues!

SWeav02 commented 8 months ago

Sounds good! I just reset now since I wasn't too far into the tests anyways. They're running now.

SWeav02 commented 8 months ago

job.32092.txt Here's the output from the tests. Some failed or weren't tested, but I'm not sure which ones are which.

jacksund commented 8 months ago

looks good! The ones that "errored" seem like it's actually just pymatgen printing new warnings for it spacegroup analyzer. Also I'm surprised the tests took 48hrs on 14 cores! Crazy. We'll have to parallelize or optimize which ones we run 😅

Do you want a v0.15.0 release this week? I'll likely be issuing a new release next week as well (so v0.16.0) -- once the quantum espresso app is more robust

SWeav02 commented 8 months ago

Yeah a new release would be super helpful. That way I can change the instructions for use on my warrenapp repo page to have users install simmate through conda/pip rather than doing a dev install.

jacksund commented 8 months ago

sweet! I'm busy with errands for the rest of today, but I'll make a release for us tomorrow!

jacksund commented 8 months ago

@SWeav02 I'm making a new minor release today (v0.15.1) for your updates. Give it some testing over the next week or two and try to iron out any bugs with your apps. Be sure to make a new env and test with the package on conda-forge to replicate your users env too.

Your fixes plus my larger changes (in the simmate config files, the new docs, etc.) will then be a couple weeks later (v0.16.0)

jacksund commented 7 months ago

I'm hoping to have the v0.16.0 release out by the end of next week (the 26th). After which, I need to pivot my focus back to private apps, and then any follow-up releases will be when you need them 😄

I'll close this issue with the v0.16.0 release too. You can open a new issue whenever a new release is needed.

SWeav02 commented 7 months ago

I'm hoping to have the v0.16.0 release out by the end of next week (the 26th). After which, I need to pivot my focus back to private apps, and then any follow-up releases will be when you need them 😄

I'll close this issue with the v0.16.0 release too. You can open a new issue whenever a new release is needed.

Sounds good. I've been working on vectorizing the badelf algorithm andI'm almost done. It should just effect the badelf class. I'll try and get it in a pull request before the 26th. so that it can be in v0.16.0