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.

Sign up for GitHub

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

split off astronomical constants from units #1059

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

split off astronomical constants from units #1059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54e26e9
split off astronomical constants from units
rieder Jun 14, 2024
3f661af
Lightyear is a physical constant (julianyr * lightspeed), not astrono…
rieder Jul 8, 2024
e8563b8
sanitize test_pickle, *don't* import * from modules! causes clashes b…
rieder Jul 8, 2024
3465180
I know, importing *... but here no better way to do it.
rieder Jul 8, 2024
a95b3e5
rewrite/modernise nist.py.
rieder Jul 8, 2024
9d20f4f
small updates
rieder Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 43 additions & 30 deletions src/amuse/test/suite/core_tests/test_pickle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,30 @@
from amuse.test import amusetest

import subprocess
import pickle
import sys
import os

from amuse.support.exceptions import AmuseException
from amuse.test import amusetest

from amuse.units import core
from amuse.units import si
from amuse.units import nbody_system
from amuse.units import generic_unit_system
from amuse.units.quantities import zero
from amuse.units.units import *
from amuse.units.constants import *
from amuse.units.units import m, km, kg, parsec, stellar_type

from amuse.datamodel import Particles, parameters

import subprocess
import pickle
import sys
import os


class TestPicklingOfUnitsAndQuantities(amusetest.TestCase):

def test1(self):
km = 1000 * m
self.assertEqual(1000, km.value_in(m))
pickled_km = pickle.dumps(km)
unpickled_km = pickle.loads(pickled_km)
self.assertEqual(1000, unpickled_km.value_in(m))
kilometer = 1000 * m
self.assertEqual(1000, kilometer.value_in(m))
Copy link
Member Author

Choose a reason for hiding this comment

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

renaming it here since 'km' is already defined... could probably just as easily use the pre-defined 'km' unit, but maybe there was a reason not to?

pickled_kilometer = pickle.dumps(kilometer)
unpickled_kilometer = pickle.loads(pickled_kilometer)
self.assertEqual(1000, unpickled_kilometer.value_in(m))

def test2(self):
km = 1000 * m
quantity = 12.0 | km
kilometer = 1000 * m
quantity = 12.0 | kilometer
self.assertEqual(12000, quantity.value_in(m))
pickled_quantity = pickle.dumps(quantity)
unpickled_quantity = pickle.loads(pickled_quantity)
Expand Down Expand Up @@ -81,15 +74,22 @@ def test8(self):

def test9(self):
quantity = 1.3 | nbody_system.time
path = os.path.abspath(os.path.join(self.get_path_to_results(), "test9.pickle"))
path = os.path.abspath(
os.path.join(self.get_path_to_results(), "test9.pickle")
)

with open(path, "wb") as stream:
pickle.dump(quantity, stream)

pythonpath = os.pathsep.join(sys.path)
env = os.environ.copy()
env['PYTHONPATH'] = pythonpath
code = "import pickle;stream = open('{0}', 'rb'); print(str(pickle.load(stream)));stream.close()".format(path)
code = (
f"import pickle;"
rieder marked this conversation as resolved.
Show resolved Hide resolved
f"stream = open('{path}', 'rb');"
f"print(str(pickle.load(stream)));"
f"stream.close()"
)

process = subprocess.Popen([
sys.executable,
Expand All @@ -100,18 +100,28 @@ def test9(self):
)
unpickled_quantity_string, error_string = process.communicate()
self.assertEqual(process.returncode, 0)
self.assertEqual(str(quantity), unpickled_quantity_string.strip().decode('utf-8'))
self.assertEqual(
str(quantity),
unpickled_quantity_string.strip().decode('utf-8')
)

def test10(self):
quantity = 1 | parsec
path = os.path.abspath(os.path.join(self.get_path_to_results(), "test10.pickle"))
path = os.path.abspath(
os.path.join(self.get_path_to_results(), "test10.pickle")
)
with open(path, "wb") as stream:
pickle.dump(quantity, stream)

pythonpath = os.pathsep.join(sys.path)
env = os.environ.copy()
env['PYTHONPATH'] = pythonpath
code = "import pickle;stream = open('{0}', 'rb'); print(str(pickle.load(stream)));stream.close()".format(path)
code = (
f"import pickle;"
f"stream = open('{path}', 'rb');"
f"print(str(pickle.load(stream)));"
f"stream.close()"
)

process = subprocess.Popen([
sys.executable,
Expand All @@ -122,7 +132,10 @@ def test10(self):
)
unpickled_quantity_string, error_string = process.communicate()
self.assertEqual(process.returncode, 0)
self.assertEqual(str(quantity), unpickled_quantity_string.strip().decode('utf-8'))
self.assertEqual(
str(quantity),
unpickled_quantity_string.strip().decode('utf-8')
)

def test11(self):
value = 1 | stellar_type
Expand Down Expand Up @@ -175,7 +188,7 @@ def test4(self):
self.assertEqual(unpickled_particles.center_of_mass(), [2, 3, 0] | m)


class BaseTestModule(object):
class BaseTestModule:
def before_get_parameter(self):
return

Expand All @@ -202,13 +215,13 @@ def set_test(self, value):
self.x = value

o = TestModule()
set = parameters.Parameters([definition,], o)
set.test_name = 10 | m
paramset = parameters.Parameters([definition,], o)
paramset.test_name = 10 | m

self.assertEqual(o.x, 10 | m)
self.assertEqual(set.test_name, 10 | m)
self.assertEqual(paramset.test_name, 10 | m)

memento = set.copy()
memento = paramset.copy()
self.assertEqual(memento.test_name, 10 | m)

pickled_memento = pickle.dumps(memento)
Expand Down
1 change: 1 addition & 0 deletions src/amuse/units/amuse_2010/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# constants as originally defined/used in AMUSE
19 changes: 19 additions & 0 deletions src/amuse/units/amuse_2010/astronomical_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
"""
Series of astronomical constants, which are in turn used by the units of the
same names.
"""

import numpy as np
from amuse.units.si import m, kg
from amuse.units.derivedsi import W, km


au = 149597870691.0 | m
parsec = au / np.tan(np.pi / (180 * 60 * 60))
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that both 'au' and 'parsec' are also defined as units... Be careful, this may clash!
For Lsun etc below, note that here they only have one capital letter, the unit has LSun etc... This is to be more in line with Astropy etc, which only have one capital letter. Still, it may be better to be consistent within AMUSE?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, that's a really subtle problem actually. Having a unit and a constant with almost the same name but different capitalisation seems like asking for trouble, because how do you remember which is which? Perhaps the best solution is to have a single object that can be used in both ways? But that would probably require some major redesign....

Copy link
Member Author

Choose a reason for hiding this comment

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

normally it wouldn't be a problem, since units would be used as units.(unitname) rather than being directly imported. But I don't exactly like it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are constants that can also be units actually ever used as anything else than units?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not. Except for making the units.

Copy link
Member Author

Choose a reason for hiding this comment

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

So... We could in principle make the names here (way) more descriptive, e.g.:

distance_parsec = ...
distance_au = ...
luminosity_sun = ...
radius_sun = ...

That would solve this issue. It's very verbose but since you're always using the unit form that should not be a problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, they could still have the same names actually, as long as no one directly uses the amuse.units.amuse_2010.astronomical_constants module. It should only be used in the modules that define the units then, and we could even name it _astronomical_constants to mark it as private.

Then in the modules where the units are defined, we could import amuse.units.amuse_2010.astronomical_constants as ac and write ac.au and the like everywhere to clearly point out where they're coming from and that these are the constants and not the units. I think that's what I'd do.

Lsun = 3.839e26 | W
Msun = 1.98892e30 | kg
Rsun = 6.955e8 | m
Mjupiter = 1.8987e27 | kg
Rjupiter = 71492.0 | km
Mearth = 5.9722e24 | kg
Rearth = 6371.0088 | km # IUGG mean radius
10 changes: 10 additions & 0 deletions src/amuse/units/astronomical_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
"""
Series of astronomical constants, which are in turn used by the units of the
same names.
"""

# Until we have a way to select a system of constants, use the originally
# defined values. Later, we should have a way to select between different
# constants definitions.

from amuse.units.amuse_2010.astronomical_constants import *
Loading
Loading