Toolkit-dependent behavior of Molecule.from_qcschema for two QCA records

Describe the bug
Molecule.from_qcschema(record, toolkit_registry=tk) returns different molecules depending on tk for two records in QCArchive "OpenFF Gen 2 Opt Set 2 Coverage" dataset.
(The two records reported here appear to be the only ones in the OpenFF 1.2 training set with this behavior.)
To Reproduce
import qcportal
from openforcefield.topology import Molecule
from openforcefield.utils.toolkits import RDKitToolkitWrapper, OpenEyeToolkitWrapper

oe = OpenEyeToolkitWrapper()
rd = RDKitToolkitWrapper()

dataset_type = "OptimizationDataset"
dataset_name = "OpenFF Gen 2 Opt Set 2 Coverage"
problem_records = ["c1c[nh+]c(cc1cl)[s@@](=o)[o-]-0", "c1[c@@h]2[c@@h]([c@h]2o)c[nh2+]1-0"]

client = qcportal.FractalClient()
ds = client.get_collection(dataset_type, dataset_name)

for record in problem_records:
    print(f"Querying QCA entry:\n\t{dataset_type}\n\t{dataset_name}\n\t{record}")

    entry = ds.get_entry(record)

    # load using one toolkit backend or the other
    mol_rd = Molecule.from_qcschema(entry, toolkit_registry=rd)
    mol_oe = Molecule.from_qcschema(entry, toolkit_registry=oe)

    # with RDKit backend, reported not isomorphic
    rd_isomorphic = mol_oe.is_isomorphic_with(mol_rd, toolkit_registry=rd)
    print(f'RDKit is_isomorphic: {rd_isomorphic}')

    # with OpenEye backend, error encountered when converting `mol_rd` to OpenEye
    try:
        mol_oe.is_isomorphic_with(mol_rd, toolkit_registry=oe)
    except Exception as e:
        print(f'OpenEye is_isomorphic: {e}')
    print()

Output
Querying QCA entry:
	OptimizationDataset
	OpenFF Gen 2 Opt Set 2 Coverage
	c1c[nh+]c(cc1cl)[s@@](=o)[o-]-0
RDKit is_isomorphic: False
OpenEye is_isomorphic: Programming error: OpenEye atom stereochemistry assumptions failed.

Querying QCA entry:
	OptimizationDataset
	OpenFF Gen 2 Opt Set 2 Coverage
	c1[c@@h]2[c@@h]([c@h]2o)c[nh2+]1-0
RDKit is_isomorphic: False
OpenEye is_isomorphic: Programming error: OpenEye atom stereochemistry assumptions failed.

A bit more detail in this notebook: https://gist.github.com/maxentile/ae2a19943181236fff471855977e963e

Computing environment:

openforcefield.__version__ == '0.7.2'
rdkit.__version__ == '2020.03.6'
openeye.__version__ == '2020.1.0'

Wow. These are really interesting. Thanks for the report. @trevorgokey and I are checking this out.
c1c[nh+]c(cc1cl)[s@@](=o)[o-]-0
I don't know what should happen here. Neither cheminformatics toolkit sees the S or its bonds as aromatic. Basically we think the S is pyramidal. But without any other stereocenters on the molecule, it's not important (for charge calculation) stereochemistry (since both stereoisomers would have identical electronic structures).
So, I think the RDKit interpretation gets this right (the S DOES have stereochemistry). If I can reproduce this using only OpenEye code, I'll bring it up on the issue tracker there.
Oddly, since OpenEye MADE the canonical SMILES (which DOES have stereochemistry) I'm not sure how it lost track of the stereochemistry here.
"c1[c@@h]2[c@@h]([c@h]2o)c[nh2+]1-0"

This is crazy interesting because the existence of the third stereocenter (the C-OH one) depends on the stereochemistry of the first two. That is, the rightmost carbon in the 3-membered ring is a stereocenter if it has 4 unique substituents. The two "arms" are certainly unique, but the two other ring carbons may or may not be unique from each other, depending on their stereochemistry. I... really don't know what the "correct" underlying toolkit behavior should be in this case. This makes my head hurt.

Want to point out some patterns here, which may be good to collect as we work through these as we try to generalize our findings. A general pattern is that RD will print wedge bonds if you put them in, no matter what. OE will not show wedges if it detects that the atom is not chiral, even if the SMILES specifies that it is chiral. Therefore RD assumes the input SMILES is correct, wheras OE does not assume it is correct. I think this is the biggest pattern that I am noticing across the board.
I tried to build these manually, using from_smiles and enumerating all possible stereocenters. I see that,
For the first molecule:
Removing all chiral tags, both OE and RD report that S is chiral, and both fail to build if allow_undefined_stereo=False.
For the second:
RD only complains about the first two centers; OE agrees. This means RD is showing it as a wedge just because the SMILES says so.
The confusing part to me here is that, given that OE doesn't see the third center as chiral, how it even showed up in the SMILES to begin with. These SMILES in the entries were generated using CMILES with OE 2019.4.2, which is what I used here. So I am not understanding how those chiral tags are getting in there if OE is not detecting them as chiral.
To be more explicit, we see that
mol = Molecule.from_smiles('C1[C@H]2[C@H]([C@H]2O)C[NH2+]1', toolkit_registry=rd)
mol2 = Molecule.from_smiles('C1[C@H]2[C@H]([C@H]2O)C[NH2+]1', toolkit_registry=rd)
mol.is_isomorphic_with(mol2, toolkit_registry=rd)
True

# flipping the center of one of them, even though it doesn't perceive it as chiral at all
mol = Molecule.from_smiles('C1[C@H]2[C@H]([C@@H]2O)C[NH2+]1', toolkit_registry=rd)
mol2 = Molecule.from_smiles('C1[C@H]2[C@H]([C@H]2O)C[NH2+]1', toolkit_registry=rd)
mol.is_isomorphic_with(mol2, toolkit_registry=rd)
False

even though Molecule.from_smiles('C1[C@H]2[C@H]([CH]2O)C[NH2+]1', toolkit_registry=rd, allow_undefined_stereo=False) works with no issue (note the third center here has no chirality defined). So, RDkit assumes we know what we are talking about :) In OE, both return True, since it implicitly drops the @ tag. This means you can get things like
mol = Molecule.from_smiles('c1c[nH+]c(cc1Cl)[S@@](=O)[O-]', toolkit_registry=oe)
mol2 = Molecule.from_smiles('c1c[nH+]c(cc1Cl)[S@](=O)[O-]', toolkit_registry=oe)
mol.is_isomorphic_with(mol2, toolkit_registry=oe)
True
mol.is_isomorphic_with(mol2, toolkit_registry=rd)
True
mol = Molecule.from_smiles('c1c[nH+]c(cc1Cl)[S@@](=O)[O-]', toolkit_registry=rd)
mol2 = Molecule.from_smiles('c1c[nH+]c(cc1Cl)[S@](=O)[O-]', toolkit_registry=rd)
mol.is_isomorphic_with(mol2, toolkit_registry=oe)
Exception: Programming error: OpenEye atom stereochemistry assumptions failed.
mol.is_isomorphic_with(mol2, toolkit_registry=rd)
False

I am not sure what to take from this. On one hand, I see RDkits side of saying the input is 100% authoritative and blindly follows the data. On the other hand, I see how one would take OE's approach and "do the right thing," but I can't understand why OE produced the SMILES it did, since it seems to be arguing with itself here.
I think there are two problems:

OE detects the sulfur as chiral, but it a. doesn't show it as chiral in the visualization, and b. says it is not isomorphic with the RD form. OE appears to be taking resonance into account and arriving to the correct result, but only after building the molecule, because it does detect it as chiral at first. The final result should be that the sulfur is not chiral, but it is really hard to judge that because the input SMILES explicitly states it as being chiral.

The third center (in the second molecule) is actually chiral, and OE's sanitation is dropping it from the input SMILES. Both miss that it is not chiral (since they both work with allow_undefined_stereo=False), but since RD respects the input SMILES at all times, it preserves the info.

Both of these problems seem to stem from the fact that the input SMILES is technically incorrect, and we are witnessing the difference in how the two TKs handle problems.

@j-wags, @trevorgokey , I don't have much to add to the observations you've described, but I want to say thank you x 100 for looking into these, and for such clear and perceptive write-ups about the behavior and possible underlying patterns. When I encountered these I had no idea where to start looking, and I'll keep in mind the patterns you identified in case we encounter similar issues in the future.

My two cents: I think OE is correct here in both cases:

The sulfur is chiral, but it will rapidly interconvert between stereo-isomers; there is no way (well, no easy way) to isolate an optically pure sample, so it is better to represent it without wedges (or, perhaps better would be to represent it as a racemate)
The far-right carbon is not chiral; there is a plane of symmetry in the xz-plane relative to the orientation of the molecule as shown in the images above.

Thanks for chiming in, @sidneypaulymer.

The sulfur is chiral, but it will rapidly interconvert between stereo-isomers

This is helpful to know. Generally, we care about stereochemistry when it is long-lived enough to be a "stable" feature of a molecule, versus "likely to change over the course of an MD simulation". This determination helps us know whether to consider stereochemistry when we're doing charge assignment, since charge methods like AM1-BCC are geometry-dependent.
It seems like all pyramidal N stereochemistry (even tertiary amines) interconverts rapidly (the slowest seem to be around 1/millisecond), so they're not "defining" stereochemistry and we'll have the OpenFF toolkit start ignoring them soon.
Pyrimidal S is more complex, and we're looking for primary literature on the inversion rates so we can know which rules to enforce. We now know that sulfurs like this one shouldn't be considered chiral, but sulfurs like that in nexium should be considered chiral.
Do you know of any good sources for rules on which pyrimidal S-containing substructures have stable stereochemistry at room temperature (say, stable for > 1 second) versus not?
cc #467 #146

The far-right carbon is not chiral

I'm not so sure about this. My current thinking is that the molecule really has 3 meaningful stereoisomers, which are hard to describe using R/S notation.

The unique stereoisomers would be

H1 is coming out of the screen the ring, H2 is going into the screen ring, and it doesn't matter where O3 is
H1 and H2 are coming out of the screen, O3 is also coming out of the screen
H1 and H2 are coming out of the screen, O3 is going into the screen

Does this make sense? I may be misinterpreting something fundamental here.


Video controls overlapping and unable to dismiss in Intro video

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 9.0.16-2
Reproducible in staging?: Y
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dylanexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1722857321756849
Action Performed:

Open the hybrid app with the newly created account
Tap Menu > Try New Expensify

Expected Result:
Video plays without any issue
Actual Result:
Video controls overlapping and unable to dismiss when video playing
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Putting this in collect as it's onboarding related

Kewl, are you waiting on something to move it External and Help wanted?

Ah sorry, I totally slipped on the labeling. Added.

Done!


Implement persistent buffers mapping

Remains to do:

Tests
Add a fence when using the VB/IB/UB to draw

Everything is done, but glMapBufferRange returns null.

Fixed


feat: building website documentation in gatsby

I hope you like the website 🤗.

You can see a demo of the website here.

@gersongams I really appreciate this PR for a better way of showing this documentation. Thank you! :100:


Smart mode

Hello, what about enabling a 'smart mode', that is adding a domain for each code in configuration, then just process when onmouseover the key icon and if we are in a known domain, let the key icon be gold color and auto copy code to clipboard.
In this mode when you are in login page then just move the mouse over the key it becomes gold then go back and paste the code.
To be smarter if remaining time is less than 8sec then just blink in other color until new code is ready then gold and autocopy.
Thank You and congratulations again for this great job!
Update: In fact that was my first idea, then thinking just a bit more, I found is more easy to just filter the results of the sites to those who match the domain (if none then list all), with that the interface is the same 1.-display the list 1.-pick the code 3.-paste.
Brief: Add domain to code list and by default filter the results for the domain (if it matches), else list all
... and put you Bitcoin address so we can tip you! because I just tryed with PayPal donation, now giving 1$ cost me another 1$ in Paypal I do not know why,  perhaps is currency conversion or new PayPal tarifs so it is not suitable for small tips (non US).

That shouldn't be too hard to do, it would just require the tabs permission to get the URL of the currently open site.
Sorry about donating, I put up my paypal because that's what's most convenient for me, but my Bitcoin address is 3Ca6o8VFQGhAY2RV3yQjvHHhHGyECendtd.

Video demo: https://youtu.be/8RTMxRJOboQ

Hello,
The video shows almost the solution, the question is, what is smart mode for,  the idea is to help and accelerate the process so this first solution only helps showing the right pick but does not accelerate.
Is there a way to pick the correct answer in one step? , say the original idea.
When the domain is known, and there is only one code for it, then just ( 'on mouse over' become gold)  passing the mouse over the key icon the key becomes gold color (internally we get auto copy) then we come back to the 2FA field and press Ctrl-C.
With this way we spare one click (show key list) and one select-copy .
Brief: aprove video solution and 'smart mode' is now downgraded to 'default mode' and think about a new 'smart mode' say 'gold key mode'
I also understand that if there are more than one key per domain the list shows various marked fields.
Cool.


multi region support for aws-api integration

:nut_and_bolt: Description: What code changed, and why?
The content team has a new, certified AWS profile that supports multi-region scanning.
This code change modifies the "aws-api" integration and scan job path to support (and encourage) this updated profile/behaviour.
Previously, we were creating one node per region for every "aws-api" integration. Now that the profile itself supports multi-region scanning, we are moving to creating one node per account. We are removing the regions filter on scan job create. The inspec-agent logic has been updated to support both the old code path (one node per region) and the new one (one node per account).
We will be recommending that users delete their old "aws-api" integration and all associated nodes and create a new one to be used with the profile.
:chains: Related Resources
https://blog.chef.io/2019/03/13/chef-software-is-the-first-cis-partner-certified-on-aws-azure-and-gcp/
https://blog.chef.io/2019/03/21/hit-the-cloud-running-with-inspec-profiles/
https://github.com/chef/automate/issues/75
:+1: Definition of Done

only one node created per account for "aws-api" integration
region filters on scan job create is removed, just one node available per aws-api integration
scans for the new style and old style both work

:athletic_shoe: How to Build and Test the Change

start up the services
add an aws api integration (use okta aws functionality to get aws creds):

curl -s --insecure -H "api-token: $token" $url/api/v0/nodemanagers -d '{
    "name": "my aws api integration with session token",
    "type": "aws-api",
    "instance_credentials": [],
		"credential_data": [
			{"key": "AWS_ACCESS_KEY_ID", "value": "value" },
			{"key": "AWS_SECRET_ACCESS_KEY", "value": "value" },
			{"key": "AWS_SESSION_TOKEN", "value": "value" }
		]
  }'

see 16 nodes added
start a recurring job against that account (every 10 min or something)
rebuild compliance, nodemanager, and ui
get new creds and create another integration
expect to see one node for this new integration
install the new aws profile (i can't link it here b/c it's not public but if anyone needs it for testing pls dm me)
run a scan job with new integration and new profile
ensure scan job is created, look at the report
go back and check on the recurring scan job running on the old integration, make sure it is still doing the right thing/executing scans and creating reports

:white_check_mark: Checklist

[ ] Tests added/updated?
[ ] Docs added/updated?

:camera: Screenshots, if applicable

still working on test updates and docs for this change.
also still haven't run through the full test case described in the pr yet.

ok, i've updated docs and tests, and run through the entire test case described above. taking this out of draft.

currently discussing node name for this:
AWS Account (aws acct id or acct alias)
vs
aws-account-acct-id-or-alias

from @jonong1972 :

More and more I am still leaning towards this format for reasons:
1.) other data values are like this across Automate
2.) if I were to filter on this name, the ease of typing a Hyphen vs. Parenthesis for the auto suggest

rebasing on master

After discussing with the UX team and doing a little research online about conventional naming standards.
We decide to stick with the hyphenated name:
aws-account-acct-id-or-alias
As mentioned above, to keep consistent across Automate, we utilize the hyphens, looking through AWS and other documentation, they name their ids with hyphens.  So just to keep an industry standard for naming nodes.


Preview Govspeak button for document new and edit views

This sends an AJAX request and renders the Govspeak body text on the page.
We decided to use a Govspeak controller because serving this ajax call is something that is not specific to documents.
We've had to update jenkins to run using Xvfb, so that Capybara can run functional tests that rely on Javascript. More details here.
trello card

Looks good. A couple of minor questions. 👍

👍


[Feature] onMapRendered Callback

Thank you so much for making this awesome plugin!
I have a suggestion and hope it's something you can add. Would it be possible to introduce an onMapRendered callback that kicks in once the map is fully rendered, rather than just when it's ready? The onMapReady is great for adding overlays but not that handy for snapshots.
I'm asking because I'd love a view-only widget that could load a cached snapshot for certain locations to boost performance. Flutter's platform view is cool but has its limits, so I'm looking at this as a workaround until the platform view is stable.
Also, waiting a few seconds before taking a screenshot isn't the best solution, especially with unpredictable network issues preventing map tiles from loading on time.
Thanks a lot for considering this. Please let me know if adding this callback is something you can do.

Thank you for your feedback and suggestion regarding the plugin.
I understand the need for an onMapRendered callback, particularly for tasks requiring precise timing, like taking snapshots. This feature would indeed be a significant enhancement to the plugin's functionality.
I want to clarify that the work on implementing an onMapLoaded event was conducted up until recently for the Android platform, facilitated by the support provided by the Naver Map SDK. This effort was relatively straightforward. However, implementing a similar feature for iOS proved challenging due to the absence of a comparable feature in the Naver Map SDK for iOS.
For those interested, my previous attempts and progress can be reviewed here: https://github.com/note11g/flutter_naver_map/tree/onMapLoaded. However, it's important to note that there are currently no ongoing efforts to further this functionality due to the encountered limitations. While this might come as a disappointment, I hope you understand that under the present conditions, advancing this feature for iOS is challenging.
I want to add that if the Naver team introduces support for this feature on iOS in the future, I am fully committed to implementing it on our end as well. Rest assured, any developments or updates in this regard will be promptly communicated.
Thank you for your support and for using the plugin. Your understanding and patience are greatly appreciated.
Have a Good Night!

and if you want to get map picture, see this function of naver map sdk naver cloud, static map service

Really appreciate your quick response!
It's great to hear about the onMapLoaded event, especially for the Android platform. I might fork your repo to try out this callback since the iOS platform view is functioning well enough for my needs, and I might not need a workaround after all.
I also wasn't aware of the static map service offered by Naver Cloud. I'll definitely check it out. It sounds like a useful feature that could serve my purposes well.
Thanks again for your dedication to improving the plugin and for the informative update. I appreciate your efforts and the challenges involved, especially with platform-specific features. Keep up the great work 👍


encode non-printable body in hex

This is an interesting idea, but I wonder if it wouldn't be more effective if you used the string repr instead: body = repr(self.serialize_body()). How do you feel about that?

Usually I'm not interessted in the body of a data frame (mostly plain text anyway), but in one of the other frame types (headers, error codes, etc).
I think this is not very readable or intuitive to understand:
>>> repr(b"\x02\x10\x32\x41\x42\x43\x44")
"'\\x02\\x102ABCD'"

where as this is much better IMHO:
>>> b"\x02\x10\x32\x41\x42\x43\x44".encode('hex')
'02103241424344'

@Kriechi That's fair. In that case, you'll need to fix the test breakage. =)

Done.

\o/


Added a Hackrobrtfest intro page and updated contributors list

Added a Hackrobrtfest_intro.html page and updated contributors.html list

Great :)


Facing an error with the latest version of inspect - 0.3.18

This error occurs when we run inspect with the latest version 0.3.18 but when we downgrade it to 0.3.14, it works fine.
Traceback (most recent call last):
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/bin/inspect", line 8, in <module>
    sys.exit(main())
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_cli/main.py", line 53, in main
    inspect(auto_envvar_prefix="INSPECT")
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_cli/common.py", line 30, in wrapper
    return cast(click.Context, func(*args, **kwargs))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_cli/common.py", line 55, in wrapper
    return cast(click.Context, func(*args, **kwargs))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_cli/eval.py", line 246, in eval_command
    eval(
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/eval.py", line 109, in eval
    return asyncio.run(
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/eval.py", line 229, in eval_async
    resolved_tasks.extend(resolve_tasks(tasks, task_args, m, toolenv))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 120, in resolve_tasks
    load_tasks(cast(list[str] | None, tasks), model, task_args)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 131, in load_tasks
    return [
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 134, in <listcomp>
    for spec in load_task_spec(task_spec, model_name, task_args)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 147, in load_task_spec
    return create_tasks([task_spec], model, task_args)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 179, in create_tasks
    tasks.extend(create_file_tasks(file, model, None, task_args))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/loader.py", line 209, in create_file_tasks
    task = task_create(task_spec, model, **task_args)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/registry.py", line 84, in task_create
    return cast(Task, registry_create("task", name, **task_args))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_util/registry.py", line 157, in registry_create
    return with_registry_info(obj(**kwargs))
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/_eval/registry.py", line 111, in wrapper
    task = task_type(*w_args, **w_kwargs)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/src/basic.py", line 11, in basic_eval
    plan=[chain_of_thought(), self_critique()],
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/solver/_solver.py", line 178, in solver_wrapper
    solver = solver_type(*args, **kwargs)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/solver/_critique.py", line 44, in self_critique
    model = get_model(model)
  File "/Users/karthikkalyanaraman/work/langtrace/langtrace-testing/pyenv/lib/python3.10/site-packages/inspect_ai/model/_model.py", line 460, in get_model
    raise ValueError("No model specified (and no INSPECT_EVAL_MODEL defined)")
ValueError: No model specified (and no INSPECT_EVAL_MODEL defined)

I think we may have resolved this in v03.19 (not yet on PyPI). Could you try installing the development version as follows:
pip install git+https://github.com/ukgovernmentbeis/inspect_ai

I had the same error! I could resolve it by installing the package develop mode. (similar with jjallaire's suggestion)
pip install -e ".[dev]"

This has now been published to PyPI so closing.

thank you