librasteve / raku-Physics-Constants

some physical constants expressed as measures with value and units
Artistic License 2.0
1 stars 2 forks source link

KPye Integration Discussion #10

Open librasteve opened 1 month ago

librasteve commented 1 month ago

Pasting from KPye email

You may recall quite some time ago we had a fairly long conversation about updating Physics::Constants with all the constants from the CO-DATA dataset. I did at the time get everything together, and it was all working well until I tried to actually use it as a module (while running "mi6 test"). I spent a little time looking at it at the time, and then put it on the back burner while I thought about it, which I did occasionally.

Since you've been working in that area again recently, while I was meant to be sleeping last night I thought I'd resurrect the code and see what could be seen; I had a theory as to cause of the problem, but with no idea how to fix it.

Turns out I was wrong—that wasn't the problem, which was just as well as I had no idea how to fix it. I have since worked out a workaround, but it will require a major rewrite of the thousand or so lines of code, and I notice that the 2022 version of the data is now available, so most of the numbers are wrong, so it will take at least a few days to get it back in to shape.

The problem:

I would like code like (and this is what I had working, except not as a module):

constant \avogadro-constant := Measure.new( value => 6.02214076e+23, error => 0, units => 'mol⁻¹', );

constant \kg-amu is export := avogadro-constant; constant \L is export := avogadro-constant; constant \Na is export := avogadro-constant;

The actual code was slightly more complicated, but that will do for the purposes of this example.

But when run as a module this gave errors about not being able to unbox a P6Opaque object on the first line. (Any ideas on how to properly fix this would be welcome, but don't spend too much time on it.)

I tried various combinations of constant/my/our and with a $ sigil rather than the backslash. I eventually came up with:

my $avogadro-constant := Measure.new( value => 6.02214076e+23, error => 0, units => 'mol⁻¹', );

constant \avogadro-constant is export := $avogadro-constant; constant \kg-amu is export := $avogadro-constant; constant \L is export := $avogadro-constant; constant \Na is export := $avogadro-constant;

which is more verbose and creates an unnecessary variable, but otherwise gives the desired results. Why this works when the original version didn't I have no idea. The real code doesn't export everything by default but lets the user have some control over what is imported.

I'm currently planning over the next few days to update all the code to this form, and update the numbers. The test suite still needs a bit of work as well. It's generated automatically from data from the NIST site. If that works then I'll decide what to do with the code. (I'll also update Math::Constants with the new values, including correcting one of the numbers which is wrong and nobody has noticed.)

Kevin.

librasteve commented 1 month ago

@kevin - I have made a new Branch for us to collaborate to build v2.0 as an integration between your work and mine. Happy to do that elsewhere is you prefer.

librasteve commented 1 month ago

first

I would say that this code

constant \avogadro-constant := Measure.new(
  value => 6.02214076e+23, 
  error => 0, 
  units => 'mol⁻¹',
);

constant \kg-amu is export := avogadro-constant;
constant \L      is export := avogadro-constant;
constant \Na     is export := avogadro-constant;

say Na;    #6.02214076e+23mol⁻¹ ±0

works fine for me - here's my version - suggest checking with the IRC / Discord groups if you need help on reproducing / debugging this

Welcome to Rakudo™ v2024.06.
Implementing the Raku® Programming Language v6.d.
Built on MoarVM version 2024.06.

second

I would say that I recently changed the export mechanism of Physics::Constant since it was taking 9sec to go use Physics::Constant

the problem was:

I wrote a post on this Raku Performance and Physics::Unit

unfortunately this moved the problem to Physics::Constant

third

the fix for Physics::Constant was

constant \avogadro-constant := Measure.new(
  value => 6.02214076e+23, 
  error => 0, 
  units => 'mol⁻¹',
);

constant \kg-amu is export := avogadro-constant;
constant \L      is export := avogadro-constant;
constant \Na     is export := avogadro-constant;
my &avogadro-constant = sub {
  Measure.new(
    value => 6.02214076e+23, 
    error => 0, 
    units => 'mol⁻¹',
  );
}

my &kg-amu is export = avogadro-constant;
my &L           is export = avogadro-constant;
my &Na        is export = avogadro-constant;

say Na;     <== now this first use of the sub 'Na' calls the sub that makes and returns the Measure instance

(this example is not tested)

lastly

Sorry for all the detail. But I think that the best way to proceed is for you to point me to your current implementation and I can port over your work to here and integrate with this approach so that we can keep the performance in the ~1sec level

Of course them we can review together and agree on the shape of final v2

ok?

PS. I think it would be a good idea for v2.0 to drop the dependency on Math::Constants, personally I would like to convince the authors to remove physics constants from Math::Constants ... but I guess we need to have v2 here ready first

kjpye commented 1 month ago

Where to begin?

The original intention years ago was to add all the constants to Math::Constants. After discussing this with JJ we quickly came to the conclusion that that wasn't the right place. When this work is completed, I plan to update Math::Constants to the 2022 values and add a pointer to Physics::Constants for other values. Perhaps those values should be deprecated at that time. When the next set of values are available in a few years then perhaps that's the time they should be deleted from Math::Constants.

The documentation for Physics::Constants says that is must be used before Physics::Measure. Is this still the case? Since the new Physics::Constants will use Physics::Measure I'm not sure this is appropriate. If Physics::Measure still needs the value of a couple of constants as it used to, it may be necessary to define local values of those constants. The tests for Physics::Constants would probably indirectly check that those values were correct.

There are currently about 350 constants defined (plus aliases). The 2022 values may have added more values; I haven't checked yet. Compiling the module currently takes me about 50 seconds. This is obviously far too long to be useful. The real question is that when this is precompiled, how long does it take to load the precompiled module?

To assist that time, and to avoid polluting the users code with hundreds of names, most of which they probably wouldn't be using, I was planning to add labels to each definition. So for example the Avogadro constant definition would be something like

constant \avogadro-constant is export (:DEFAULT :avogadro-constant) ...

and the only constants loaded by default would be those currently defined in the old Physics::Constants, but a user could load the definition of any constant they needed.

More to come...

librasteve commented 1 month ago

The original intention years ago was to add all the constants to Math::Constants. After discussing this with JJ we quickly came to the conclusion that that wasn't the right place. When this work is completed, I plan to update Math::Constants to the 2022 values and add a pointer to Physics::Constants for other values. Perhaps those values should be deprecated at that time. When the next set of values are available in a few years then perhaps that's the time they should be deleted from Math::Constants.

I agree

The documentation for Physics::Constants says that is must be used before Physics::Measure.

Physics::Constants already uses Physics::Measure. When you want to use both, then you need to use them in the order stated to avoid a use loop ... so this shoudn't need to change.

There are currently about 350 constants defined (plus aliases). The 2022 values may have added more values; I haven't checked yet. Compiling the module currently takes me about 50 seconds. This is obviously far too long to be useful. The real question is that when this is precompiled, how long does it take to load the precompiled module?

This is largely addressed by my recent change i.e. to export constant symbols as subroutines. If we do this, then all the Measure.new() calls are done lazily as the particular constants are used.

I propose that you provide the data as a raku datastructure and I will adapt the working export as sub dynamic exporter to do the rest. You may prefer to do that in a yaml file for maintainability - if so, I have some examples already working over at Physics::Unit we could adapt... https://github.com/librasteve/raku-Physics-Unit/blob/main/resources/Unit/Definitions/en_SI/general.yaml

To assist that time, and to avoid polluting the users code with hundreds of names, most of which they probably wouldn't be using, I was planning to add labels to each definition. So for example the Avogadro constant definition would be something like

I like the idea of labels (although I would just import all so hopefully this would be the default still). I suggest domains such as <chemistry astronomy ...> if that makes any sense?

Anyway, just point me to your latest version of the constants and - if you are happy to collaborate here - I can bring them over to a branch here and test the export logic for you to then review...