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

dlib::expected #2827

Closed
wants to merge 74 commits into from
Closed

dlib::expected #2827

wants to merge 74 commits into from

Conversation

pfeatherstone
Copy link
Contributor

WIP.
This needs a crazy amount of unit tests.
This should be a language feature given how complicated it is to implement correctly.

@pfeatherstone
Copy link
Contributor Author

@davisking Writing our own unit tests for this might be a ball ache. Do you think we can copy MSVC's unit tests and refactor to make it work with dlib's unit testing framework?:
https://github.com/microsoft/STL/blob/main/tests/std/tests/P0323R12_expected/test.cpp
https://github.com/microsoft/STL/blob/main/tests/std/tests/P2505R5_monadic_functions_for_std_expected/test.cpp

MSVC STL's license

The Microsoft C++ Standard Library is under the Apache License v2.0 with LLVM Exception:

                                 Apache License
                           Version 2.0, January 2004
                        http://www.apache.org/licenses/

@arrufat
Copy link
Contributor

arrufat commented Jul 16, 2023

Maybe you can ask ChatGPT what tests should be used for a self written std::expected in C++. ChatGPT is a license laundry system :P. Just kidding.

@davisking
Copy link
Owner

Maybe you can ask ChatGPT what tests should be used for a self written std::expected in C++. ChatGPT is a license laundry system :P. Just kidding.

Yeah and I don't trust it to be able to make sensible tests anyway :)

Although ChatGPT is super amazing. Still though, making tests is nearly the same thing as designing and understanding an interface contract. Which is far and away the hardest part of making software. It's also part of the "so what do you want the computer to do?" question. Which somehow has to be communicated to the computer (or even other human). It's unknowable by a third party what the tests should even be without an interface contract.

What happens often though is people tacitly some contract (just silently in their heads) while editing the code. I've seen plenty of that in real life (I'm sure everyone has). You get a bunch of people looking at a function's name and bringing their assumptions along for the ride to tacitly assume some contract. Invariably different people assume different contracts and then there are bugs when those contract definitions don't align. E.g. person A writes code calling our undocumented function assuming the contract is one way and person B writes code assuming the contract is some different way. And maybe it's working fine at the moment. But then person C comes along, assumes some 3rd contract, and now there are 3 different bits of software depending on the undocumented thing. Invariably someone made a mistake at some point and now things are not working well (maybe it's seg faulting). So one of the three people (or even a 4th) comes along and "fixes" the undocumented function. Their fix assumes some contract, which could be any of the three noted or some 4th different one. And maybe that new contract they implemented the fix with respect to is now incompatible with one of these other silently assumed contracts so now there is some new bug in the code using this stuff. Which will surface later in some confusing way and it won't even be clear which code is "wrong". Since depending on the contracts you want to say things uphold it could be the client code that's wrong or it could be the library code that's wrong. And it just spirals out of control from there until no one can understand what changes will or won't break the system because each bit of code makes incoherent assumptions about what the contracts of the other parts of the system are.

Anyway, contracts are life :D

@davisking
Copy link
Owner

ChatGPT is amazing though. I love it.

You all try to play tic tac toe with it though? It can't do it. You get it onto something that isn't well represented in the training data and it just can't reason about it if it involves any non-trivial number of steps in the reasoning process to do it.

@arrufat
Copy link
Contributor

arrufat commented Jul 16, 2023

Yes, I also like ChatGPT. Some of my latest PR may or may have not been drafted with it.

It wasn't a one-prompt thing experience, though… It knows many things about dlib, but it makes up a lot of stuff. I couldn't get it to produce fully working code for that PR, so I had to step in. But it was a fun and amazing experience.

@davisking
Copy link
Owner

Yeah. It's real nice for API reference questions. Like "what's the function for doing X again?". I've also used to find out random stuff like, what episode of star trek did they decide you couldn't go faster than warp whatever? It's got a bead on pretty much everything. Not always right but more right than a vanilla google search most of the time.

@pfeatherstone
Copy link
Contributor Author

I don't have much interest in large LLMs and training architectures I can't train myself on my own datasets to solve real world problems. I could be wrong, but you need a huge cluster, lots of money, a dataset the size of the internet and possibly 100s of humans to train the damn thing. It's cool but only a handful of organizations can work on this. I'm looking forward to TinyCorp releasing commodity hardware.

@pfeatherstone
Copy link
Contributor Author

I find it frustrating that even a 100 million parameter transformer requires at least 2 large GPUs like the A6000.

@davisking
Copy link
Owner

Oh yeah, it costs a ton of money to train something like ChatGPT. It is well out of reach of one person doing it on their computer on the side. Just the costs in electricity are enormous.

You should play with it though. It's still really cool. Like I've got this nice large flat screen TV. I could never make that on my own but I still love it :D

@pfeatherstone
Copy link
Contributor Author

TODO: There are some strong exception guarantees that need to be satisfied.

…s will be bothered to review this. This isn't a 2 hour job.
Copy link
Owner

@davisking davisking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b880675 an extra assignment operator. jesus, this is a headache. I doubt Davis will be bothered to review this. This isn't a 2 hour job.

Na I always review 😭

- trying to fix with gcc 7.5. Willing to bet there is a bug in gcc7.5
@davisking
Copy link
Owner

So I tried moving all the implementation details at the bottom. In order to do that I have to do a lot of forward declaration and splitting declarations from definitions. I could do that but given that the user never really has to read this file, only cppreference, I'm reluctant to do a massive refactor. I've added the block comment at the top of the file that explains what objects are implemented and points to cppreference for docs.

Otherwise, I've pretty much copy pasted unit tests from MSVC's unit tests. You can never have enough unit tests. The green button is life.

Cool, yeah don't do any refactors. Just some comments at the top is good.

@pfeatherstone
Copy link
Contributor Author

I've added some tests from MSVC. Lots of stuff doesn't work. And i've commented out a whole bunch of static_asserts. Pfff, starting to think life is too short. Just when you think you know a language, you find out you can't even write a fancy std::pair. WTF

@davisking
Copy link
Owner

Yeah it's got a hefty amount of corner cases.

@pfeatherstone
Copy link
Contributor Author

So I've added all the MSVC tests. I've had to disable a lot of static_assert statements. It could be that a couple noexcept specifiers are slightly off. Otherwise very close. This is hard work.

@pfeatherstone
Copy link
Contributor Author

the error windows-latest is complaining about I don't get. I wonder if that is a legit bug with MSVC and/or their standard library.

@dlib-issue-bot
Copy link
Collaborator

Warning: this issue has been inactive for 35 days and will be automatically closed on 2023-11-10 if there is no further activity.

If you are waiting for a response but haven't received one it's possible your question is somehow inappropriate. E.g. it is off topic, you didn't follow the issue submission instructions, or your question is easily answerable by reading the FAQ, dlib's official compilation instructions, dlib's API documentation, or a Google search.

@pfeatherstone
Copy link
Contributor Author

I'll get round to this at some point. Not sure if many people will use it though. so maybe not worth the effort.

@davisking
Copy link
Owner

I'll get round to this at some point. Not sure if many people will use it though. so maybe not worth the effort.

Yeah I wouldn't bother with it unless you are personally jazzed about doing it :)

@pfeatherstone
Copy link
Contributor Author

Yeah let's just close it. No one cares. It was educational at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants