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

[ARTS3] Mocking scattering data interface. #719

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

simonpf
Copy link
Contributor

@simonpf simonpf commented Jan 24, 2024

This PR aims to sketch out the ARTS integration of the new scattering data handling.

@simonpf simonpf marked this pull request as draft January 24, 2024 04:19
@simonpf
Copy link
Contributor Author

simonpf commented Feb 14, 2024

@riclarsson As per out discussion from last week, I started replacing ParticulatePropertyTag with ScatteringSpeciesProperty as defined in src/core/scattering/properties.h. Could you have a look a that as well as the changes in atm.h? Thanks!

@riclarsson
Copy link
Contributor

Nice!

Two things. The first serious, the second a minor design choice (that also has a bug).

The big problem first:
How does scattering, the CMake library, depend on artscore? I really think it is a bad idea to do this. We want less cross-dependencies to make all this work nicely in the end. (At some point, artscore might also depend on scattering since we want some internal functionality using multiple parts of core ARTS functionality. So it is expected to cross-pollinate, which could cause issues.)

The second minor thing:
I think there's a bug in ParticulateProperty toParticulatePropertyEnumOrThrow(const std::string_view x); The .cc file defines ParticulateProperty toParticulatePropertyOrThrow(std::string x);. The ENUMCLASS macro gives you ParticulateProperty toParticulatePropertyOrThrow(const std::string_view x); already. For SpeciesEnum, we also want short names, like you have in the .cc file. We definitely want short-names for SpeciesEnum, but are they really a requirement here? (Several people of RTE community will much more easily recognize C2H2 than Acetylene, but I cannot make C2H2 the enum name because we must support things like NO+, which isn't a good variable name [and workarounds like NOplus will add more confusion]. I do not see this as such a big deal in the ParticulateProperty enum, at least not with the names you have there today.)

bool operator==(const ScatteringSpeciesProperty& other) const {
return (species_name == other.species_name) &&
(pproperty == other.pproperty);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

constexpr auto operator<=>(const ScatteringSpeciesProperty& other) const = default;

@riclarsson
Copy link
Contributor

@simonpf This does not compile so merging main arts3-dev is not good. There seems to be some problems with duplicate psd.cc files. We have to hold merging until we can compile it. (Or we will probably introduce even greater bugs.)

@olemke olemke changed the title Mocking scattering data interface. [ARTS3] Mocking scattering data interface. Mar 7, 2024
@simonpf
Copy link
Contributor Author

simonpf commented Mar 13, 2024

So, this should in principle build. Could you add fftw3 to the build environment @olemke ?

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

Successfully merging this pull request may close these issues.

None yet

3 participants