ExtremeFLOW / neko

/ᐠ. 。.ᐟ\ᵐᵉᵒʷˎˊ˗
https://neko.cfd/
Other
159 stars 27 forks source link

Explicitly list all types in factories for friendly error messages #1204

Closed timofeymukha closed 2 weeks ago

timofeymukha commented 3 months ago

This is a suggestion to enhance all our json-based factories with a KNOWN_TYPES list with the names of all the types the factory can create. We can then output this list when an unknown type is provided by the user in the case file.

This has proven to be a nice way of exploring what is available in the code in e.g. OpenFOAM. One intentionally puts a jibberish string in the "type" just to see the options. The minus is a bit of extra burden on the developers, but I think it is tolerable.

So far "don't merge" as I will extend it to all factories upon approval.

Question: is the string concatenation routine ok? Will it just reallocate the result to the necessary size as we are concating in new items to it?

timofeymukha commented 3 months ago

Besides for the character thing, are we in agreement for extending this to other types?

timofeymukha commented 2 weeks ago

@njansson @timfelle @MartinKarp The PR is now open for review. In addtion to adding the KNOWN_TYPES, in indroduced the following name convention:

njansson commented 2 weeks ago

There's several changes in the BSD license text, probably from some refactoring, which needs to be reverted

timofeymukha commented 2 weeks ago

BSD license text shall not be altered

I knew using %s/ will bite me in the butt :-)

timofeymukha commented 2 weeks ago

@njansson NB: ifx just got fixed by commenting out the private statements I added to the precon and ksp factory modules. I think we should try to clean up the use statements with respect to these types, there seems to be a lot of implicit propagation going on via just using the factory module.

njansson commented 2 weeks ago

@njansson NB: ifx just got fixed by commenting out the private statements I added to the precon and ksp factory modules. I think we should try to clean up the use statements with respect to these types, there seems to be a lot of implicit propagation going on via just using the factory module.

Great, but what a mess (sigh). I through that we had cleaned these in the past.

timofeymukha commented 2 weeks ago

@njansson NB: ifx just got fixed by commenting out the private statements I added to the precon and ksp factory modules. I think we should try to clean up the use statements with respect to these types, there seems to be a lot of implicit propagation going on via just using the factory module.

Great, but what a mess (sigh). I through that we had cleaned these in the past.

We did to some extent, but not thoroughly enough perhaps :-). Although, for individual ksp types the use seems to be pretty damn clean, almost everything behind only.

Hang on, would that not just suggest that some other piece of code did not do their module importing correct, and not that private is bad. (Again this might be better suited for a separate PR) / Tim F.

Probably. I did fix those though, and the other compilers where happy. I uncomment the ksp one now to test if we can get a way with just that one :P. / Timofey

Update: it worked. So the issue is pinpointed to the precon. /Timofey

timofeymukha commented 2 weeks ago

Should be all good now, I think.