Results without Patch loss

Hi,
This paper looks very interesting.
I'm wondering whether the patch loss is a necessary component of this method or it would work without this loss as well.
In the figures 11/12 you do a qualitative comparision against VolSDF and NeuS but they don't have this extra patch loss.
Wouldn't it be better to compare against SparseNeuS?
Thank you!

Hello,
The patch loss is not necessary. We don't enforce the patch loss for the open surfaces, since the patch loss assumes the surfaces are continuous.
The patch loss is only included in the training of closed surfaces (DTU here), and you can see the figure.6, the patch loss can improve the reconstruction results.  Without the patch loss, our method also works well.
SparseNeuS is a generalizable work targeting sparse views, which is different from our setting.


Add water models

[ ] Make subdirectory of water models discoverable
[ ] Add water models

[ ] TIP3P-FB

I think the TIP3P-FB file could use a version string - Maybe tip3p-fb_0.0.1.offxml or 1.0.0. This will ensure that we have a path to fixing issues with "our SMIRNOFF port"/"interpretation" of the external FF.

As an update: We didn't resolve the open questions about this plan at yesterday's ff-release meeting. Lily, Diego, and I are going to try again with a smaller group that has defined authorities at Monday's lead team meeting. The two details that seemed to resolve for me are:

Most, if not all, FF ports and external FFs will very likely live in a separate repo that openFF explicitly states are not supported
Stakeholders are split as to whether water models can be shipped alone, or if they must be shipped with ion parameters. Stakeholders are further split over whether these ion parameters should include monovalent ions, divalent ions, and coordinated ions/metals.


Latest Release/Tag

Is it possible for a tag of the latest to be created? php70 work was merged but no release or bump to metadata.rb

Planning to do that later today. Thanks for asking!

Thanks alot! :smiley:

This has been released with v1.1.4 of the cookbook. Thanks :)


Question installing cmake-built shared library to fixed destination in editable mode

Hi there, first of all thank you for all the amazing work.
I'm currently maintaining some python projects that bind to c++ code in a shared library via the python ctypes module. I have been using scikit-build with a setup.py file to build the c++ shared library and then place it relative to the python source files where the package can find it during import.
I've been working on migrating from scikit-build to scikit-build-core and have been able to get the project installing correctly by setting the output destination of the shared library file relative to ${SKBUILD_PLATLIB_DIR}. However when the package is installed in editable mode such as during development this does not copy to shared library file to the same location relative to the python files.
Is it possible to detect in cmake when the project is built in editable mode and change the output destination accordingly? Alternatively, is there some better way for me to place and find this shared library file that I'm completely missing (tried importlib.resources but no luck)?

In spglib, I simply install directly to the relative path, and they I just setup wheel.install-dir. I am not sure if this is the intended installation method, but it does work with editable mode.

Is it possible to detect in cmake when the project is built in editable mode and change the output destination accordingly?

Iirc, editable.rebuild will trigger a cmake rebuild whenever you import the python package. One way to ensure you are always pulling the correct version of the python bindings is to link only to static libraries (the python binding should still be a MODULE as far as I understand now). Otherwise scikit-build-core should add an appropriate value for CMAKE_INSTALL_RPATH. (In spglib I implemented an unconventional approach to prioritize linking to the system library. Maybe there's a cleaner way to do that though).
Hope the tips help.
PS: can you link me to the project? I am looking for some inspiration for C++ python binding that has proper usage of classes and std library.

Hey @LecrisUT thanks for the suggestions, I took a look but no immediate luck, I'll try some of them again tomorrow.
The two projects I'm trying to migrate to scikit-build-core are llama-cpp-python (currenty scikit-build with a setup.py file) and ggml-python (scikit-build-core with a workaround that copies shared libary to both valid locations). In both projects trying to build a shared library in a git submodule where submodule already has a build target I can reference from the CMakeLists file. I have interfaces written using ctypes so I don't need any seperate C / C++ source files in my own projects.

Ok, I kinda get the gist of what you do there with llama-cpp-python. That one is similar to how I load the spglib C library. I see that you are installing to the llama_cpp package folder, so you can try with an empty wheel.install-dir. If that doesn't work, put scikit-build-core in debug mode and see where cmake installs to and where sk-core moves them to.

I hope you don't mind me commenting, but after some thought I think it's dangerous to use a direct call to import the libraries and directly call and assume the structure of the cpp library. Mostly because there is only linking to the file name and it is not linked to the library version. When the libraries are linked between each other, the soname, version etc. are baked in the linkage (not sure how to do version specific linkage in cmake though).
If you are using a python binding module file, then you are guaranteed to:

be loadable by import syntax. I think the python abi is the key part for doing that
the linked cpp library is automatically loaded using the OS rules, e.g. LD_LIBRARY_PATH
you have a native python interface without any assumption on the library linked to
all of the #define are baked in

The downside to making a normal module instead is that then you have to build one wheel per python version, while the ctypes version doesn't care about what Python version you are using.
Does your method (loading the so directly) work when run through auditwheel? I'm guessing it doesn't mangle the names of the shared SO, and instead you'd just not be able to bundle your lib into two different wheels?
In general, you should never rely on the file layout. Python happens to default to laying out things on a file system, but it's not required. It could be inside a zip file. I've even seen people use a database instead of a real filesystem (common in banking, IIRC). Instead, you should always use importlib.resources, and I think that should still work in editable mode. I think. I'll try to look at what you are doing soon.

Do you have a PR or branch with your attempt that mostly works except for editable mode on one of the projects? (whichever one is simpler :) )

@abetlen, here is a snippet from spglib when I run pip install -e .
install({'spglib.spglib': '/home/lecris/mpsd/Projects/spglib/python/spglib/spglib.py',
         'spglib': '/home/lecris/mpsd/Projects/spglib/python/spglib/__init__.py'},
        {'spglib._spglib': 'spglib/_spglib.cpython-311-x86_64-linux-gnu.so',
           'spglib.spglib': 'spglib/spglib.h',
         'spglib.libsymspg': 'spglib/libsymspg.so.2.1.0'}, None, False, True, [], [])

Notice how spglib.libsymspg is being aliased. I believe that if you use importlib.resources it will use the alias when running in editable mode.
But the the question is how do you use importlib.resources to prioritize the system installed one over the bundled one?
Also this use-case will be nice to add as an example @henryiii once all the nuances are understood.

Ah, we might not support importlib.resources properly yet. I tried something that should work and it's not loading the correct file, so I think we need to implement get_resource_reader.

Thaks @LecrisUT and @henryiii
I aggree that the current solution is probably brittle and having the additional resolution logic included in imports is prefferable, I might consider using pybind11 here if there's an easy way to re-export the existing C APIs without too much boilerplate.
Currently the libraries I'm linking against (llama.cpp and ggml) don't have stable releases / APIs so it doesn't make sense really to search LD_LIBRARY_PATH. In terms of wheels, I'm actually only releasing source distributions, this is because the libraries make use of very specific performance optimizatoins which I can't build individual wheels for.
The ggml-python main branch has my best attempt at migrating to scikit-build-core there's currently a hack workaround where I copy the ggml shared library to both the SKBUILD_PLATLIB_DIR
and CMAKE_CURRENT_SOURCE_DIR (to get it working for editable model).

if there's an easy way to re-export the existing C APIs without too much boilerplate.

That would be swig and there is an example provided ;). Just make an appropriate input/header file that contains all of the apis you want to export to python.

In terms of wheels, I'm actually only releasing source distributions, this is because the libraries make use of very specific performance optimizatoins which I can't build individual wheels for.

Indeed that seems like a reasonable implementation that you had there. Hopefully my spglib example is useful to show how to achieve that with python modules since my goal there is the same. If you find anything along those lines let me know about it as well.

Yes, if you are not using the one binary for all Pythons feature, I'd probably go for something like swig, pybind11, or nanobind.
Though I'll try to work on making importlib.metadata work soon(ish).

@henryiii I appreciate it, for now I'll keep my ctypes approach with the workaround of copying the shared library to both locations but I have moved over to using importlib.resources to get the full file path instead of expecting it to be relative to the python files.
I've also started experimenting with swig (early) and I'll give pybind11 / nanobind a try as well.

(privateGPT) C:\Users\AJAY\Desktop\PrivateGpt>$env:CMAKE_ARGS = "-DLLAMA_BLAS=ON -DLLAMA_BLAS_VENDOR=OpenBLAS"
The filename, directory name, or volume label syntax is incorrect.
(privateGPT) C:\Users\AJAY\Desktop\PrivateGpt>pip install llama-cpp-python
Collecting llama-cpp-python
Using cached llama_cpp_python-0.2.20.tar.gz (8.7 MB)
Installing build dependencies ... done
Getting requirements to build wheel ... done
Installing backend dependencies ... done
Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: typing-extensions>=4.5.0 in c:\users\ajay\anaconda3\envs\privategpt\lib\site-packages (from llama-cpp-python) (4.8.0)
Requirement already satisfied: numpy>=1.20.0 in c:\users\ajay\anaconda3\envs\privategpt\lib\site-packages (from llama-cpp-python) (1.26.0)
Collecting diskcache>=5.6.1 (from llama-cpp-python)
Using cached diskcache-5.6.3-py3-none-any.whl.metadata (20 kB)
Using cached diskcache-5.6.3-py3-none-any.whl (45 kB)
Building wheels for collected packages: llama-cpp-python
Building wheel for llama-cpp-python (pyproject.toml) ... error
error: subprocess-exited-with-error
× Building wheel for llama-cpp-python (pyproject.toml) did not run successfully.
│ exit code: 1
╰─> [20 lines of output]
*** scikit-build-core 0.6.1 using CMake 3.27.7 (wheel)
*** Configuring CMake...
2023-11-30 18:40:30,114 - scikit_build_core - WARNING - Can't find a Python library, got libdir=None, ldlibrary=None, multiarch=None, masd=None
loading initial cache file C:\Users\AJAY\AppData\Local\Temp\tmpkh6aqsh2\build\CMakeInit.txt
-- Building for: NMake Makefiles
CMake Error at CMakeLists.txt:3 (project):
Running
     'nmake' '-?'

    failed with:

     The system cannot find the file specified

  CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
  CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
  -- Configuring incomplete, errors occurred!

  *** CMake configuration failed
  [end of output]

note: This error originates from a subprocess, and is likely not a problem with pip.
ERROR: Failed building wheel for llama-cpp-python
Failed to build llama-cpp-python
ERROR: Could not build wheels for llama-cpp-python, which is required to install pyproject.toml-based projects


JsonOutputParser returns empty json when text ends with ""

Checked other resources

[X] I added a very descriptive title to this issue.
[X] I searched the LangChain documentation with the integrated search.
[X] I used the GitHub search to find a similar question and didn't find it.
[X] I am sure that this is a bug in LangChain rather than my code.
[X] The bug is not resolved by updating to the latest stable version of LangChain (or the specific integration package).

Example Code
Code
from langchain.schema import Generation
from langchain_core.output_parsers import JsonOutputParser

print(JsonOutputParser().parse_result([Generation(text='{"key":"value\\')]))

Actual result:
{}
Expected result: not sure
Error Message and Stack Trace (if applicable)
No response
Description
This problem occurred when I was streaming a JSON response containing new lines "\n". The chunk stopped right after the backslash, leading to an empty json after the parser.
System Info
langchain==0.1.14
langchain-core==0.1.40
plaform: linux
Python 3.11.0rc1

Let me see.


Cargo version pinning improvements

We're still not aggressive enough about making Cargo not change stuff underfoot. Plan:

[ ] pass --locked to all cargo commands in every place we do anything in CI
[ ] set all Cargo.toml version specs of our own to = constraints
[ ] add a make update-deps target that does cargo +nightly -Z minimal-dependencies update or so
[ ] enable cackle and cargo-deny on core
[ ] update core's and env's deps using the same make update-deps and carefully compare solutions match
[ ] check that core and env both use cargo resolver v2

Not going to do all of this -- we'll do the parts that represent "locking what we've got" (steps 1, 2 and 4) but not adopt minimal-deps or change resolver or anything like that.

Why won't we change the resolver? It's a one line change.

Because it'd be weird / misleading to say we're on resolver 2 but not re-resolve with it, and the point here is to not change any more deps at this point.

Done all we're going to do here as of https://github.com/stellar/stellar-core/pull/4061 and https://github.com/stellar/rs-soroban-env/pull/1257


Small screen: Text disappears on news page upon hovering over images

Steps

Head to http://centerforopenscience.org/news/
Make your screen smaller (less than half your screen width)
Hover over the images
Notice the news text next to the images

Expected behavior
I expect the text next to the images to persist.
Actual behavior
The actual news text disappears and there are two large blue bars upon hovering over an image on the page (one on top, one on bottom).
Screenshot
Before hover

After hover

One thing with this issue, you can't "hover" on a phone or tablet, so this is not seen on those devices.

@caneruguz ... can you look at this while you are in the other?


feat: basic support for SQL SELECT -> ExtendedExpression

This allows to compile basic SQL SELECT statements to ExtendedExpressions,
the goal is to handle pyarrow Dataset support for ExtendedExpressions
in Projections and Filtering of Datasets.
This can act as a starting for future support of more advanced features,
like subqueries and entire Substrait Plans,
for the moment the goal is just to support what pyarrow.Expressions
can represent

I just imported this from an experiment, it's far from being done. I need to refactor and further improve it.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


Tim Murphy to resign on Oct 21

Just queuing this up to merge on Oct 22.

:+1: from me. For reference, the build error is that the retirement entry has an end date in the future but is in the historical file., which is correct. It'd be nice to kick the build to run again before merging so that it's clean.

Great.
Just want to note let's NOT merge until after he actually resigns, so on the 22nd (i.e. assuming the resignation is COB of the 21st). :)

Yep, that was my assumption, and was just noting that for when it is finally merge-time.

Here's his resignation letter in the Congressional Record; it was submitted yesterday. Just says "effective October 21": https://www.congress.gov/congressional-record/2017/10/10/house-section/article/H7895-7


snow effects causing a slow rendering

snow effects causing a slow rendering on the Christmas page. Triggering unresponsive browser navigation routing for few seconds.

decided to not implement snow effect due to > INP


ui-codemirror doesn't update the model

Per the instructions, I got the ui-codemirrror directive up and working.  I used the ng-model directive to bind it to a property on the scope.  This works great.
However, the ui-codemirror directive does not appear to be modifying the model when the user interacts with the editor.  For example, I can set the scope property (e.g. $scope.demoString)  to be "Hello World" when the controller initializes, and when I load the page the editor widget shows me the text.  Then I change the text to add punctuation, so it reads "Hello, World!!!".
The model, $scope.demoString , does not change to reflect these new changes.
In my view, either the directive needs to be fixed so that it updates the model when the document is changed; or the documentation needs to be updated to clarify the section re: "play nice with ng-model"; explaining to the user how to get the data out of the document.

Same here. Didn't understand what "play nice with ng-model" really means.

Same here. change textarea to div. it seams only solution now

Same here for Codemirror version 5.3, updating the view doesn't update the model. This library is broken.

And same here as well. Any possible solutions?

I use as this:
<ui-codemirror ui-codemirror-opts="editorOptions" ng-model="xml" ui-refresh="changed()"></ui-codemirror>

when I modify on codemirror board, the changed method in controller is triggered, and $scop.xml is also changed.

+1. It doesnt update model value when used with textarea. Div works fine though.

I have found it does work if the value isn't just a property on the scope. For example:
This does not work for me:
$scope.value = "// some javascript";

This does:
$scope.model = {
     value : "// some javascript"
};

Using the second code sample above, and the following HTML, the model is updated as expected:
<ui-codemirror ng-model="model.value"></ui-codemirror>

@Rohland I just realized about this. https://github.com/angular/angular.js/wiki/Understanding-Scopes ng-model requires variable.field in order to achieve dual data binding due JS restrictions.

thanks @Rohland your solution works perfectly.


Small fixes

Just a couple of small typos, style fixes, and even a minor possible performance improvement around header map generation.

I fixed some of these changes in my most recent PR, closing as not needed.


Regex support

[x]  I have looked at the documentation here first?
[x]  I have looked at the examples provided that may showcase my question here?

Enhancement:
Why dont you add a new regexp tag like so
type Country struct {
	Name   string `validate:"required" regexp:"^.{1,100}$"`
	Alpha2 string `validate:"required" regexp:"^[a-zA-Z]{2}$"`
	Alpha3 string `validate:"required" regexp:"^[a-zA-Z]{3}$"`
	Region string `regexp:"^Europe$|^Africa$|^Americas$|^Oceania$|^Asia$"`
}

I know that it won’t be possible to add support for regexp inside the validate tag and maybe this is against the design, but regexp is also a basic tool that everyone needs.

One of the unfortunate things here is that this will only allow top level validation using regex.
One of the more useful regex use-cases that I can think of is validating a map's keys and or values to match a specific regex pattern.
That said, I personally disagree with the lack of regex validators in general and think there should at-least be some way to use regex. I suspect an un-reasonably large chunk of users of this library have added their own regex validator implementations (myself included).

Hey all, it seems like that this has come back around and more people are requesting regex support again and so feel the need to comment about why it doesn't exists in the base package and also why it likely continue to remain as-is.

Many supported characters for regexes conflict with the parsing tag internals of this package, off the top of my head comma(,) and pipe(|) which already require special actions in validation params. With regexes only more case like this will crop up and outs handcuffs on what additions and changes can be added to this package without inadvertently making
a breaking change.

Having A regex tag hurts usage of other features down the line such as translations and reporting back more than just the validation failure to users. For example you could have have the message Validation failed for Name on the regex tag(not very useful in knowing why) or with registering a specific validation, backed by a regex still, Validation failed for Name on min_name_length tag as an example.

Adding a regex tag would require adding some sort of mutex guarding and compiling which can cause a small amount of contention but also the possibility that a regex fails to compile at runtime when neither is strictly necessary to use regexes through registration of other tags.

They hurt the readability of code, some can get very large (thousands of characters long).

If you use the same regexes a bunch you end up copying it all over the place instead of registering a validation backed by a regex which can be reused and live, be maintained, optimized and corrected if an error/issue is found all in one place.

If you really want a regex validation tag there's nothing stopping you from adding one pretty easily.

I might be missing more but have to start my day and think that's enough to demonstrate why it doesn't exist today and likely won't be added.
I know a lot of people would like it and reach for regexes regularly but I'm truly trying trying to help by not adding it because of the above reasons for both people and this packages sake.