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

phantom and Amuse unit scaling factors don't match #948

Open
Yrisch opened this issue Apr 18, 2023 · 4 comments
Open

phantom and Amuse unit scaling factors don't match #948

Yrisch opened this issue Apr 18, 2023 · 4 comments
Labels

Comments

@Yrisch
Copy link

Yrisch commented Apr 18, 2023

Describe the bug

During the start of Phantom, The scaling factor initialising does not take the converter value..
It always chooses 1 Myr in second 1 Msun and the the corresponding length to match G=1 instead. It does not interfere with the conversion between Amuse and Phantom but if I try to use physical units in Phantom, Particles pos/vel/internal energy will be in the wrong scaling.

Ps : Strangely, the set_unit is correctly made before the call of set_unit_length/mass/time. I can't find these three callsl in the source..

To Reproduce
Steps to reproduce the behavior:
The converter is fixed in the Phantom interface with values (0.1 pc ,1Msun, and the corresponding time to match G=1)
Setup a Phantom object. during the gas_particles.add_particles the code run setters for ulength,umass,utime.
you'll see the result in the Phantom log file.

Ps : add "print*,utime,udist,umass" in amuse_set_unit_length before any change.

Expected behavior
the setter should set the right value.

Logs

Using NATIVE inverse sqrt

Running on AMD Ryzen 7 4800HS with Radeon Graphics
16 cpus @ 3.83 Ghz, cache size 512 KB
Running in openMP on 16 threads
dtmax: 5.3012566233989994E-004
Ps : print of udist/time/mass in set_unit_length : 3.0860000000000000E+017 14881140623620.354 1.9891000000000000E+033
set_unit_length called: utime/mass/dist: 14881140623620.354 1.9891000000000000E+033 5.0936369248881318E+017
set_unit_mass called: utime/mass/dist: 14881140623620.354 1.9889199999999999E+033 3.0859069098660026E+017
set_unit_time called: utime/mass/dist: 31557600000000.000 1.9889199999999999E+033 5.0936369248881293E+017

Environment (please complete the following information):

  • OS and version: [e.g. popOS 20.4]
  • Compiler: [e.g. gcc9.4]
@Yrisch Yrisch added the bug label Apr 18, 2023
@Yrisch
Copy link
Author

Yrisch commented Apr 18, 2023

Erratum : the Post-scriptum set_units is made manually with the good value in amuse_initialize_code, so not related with the converter value.

@Yrisch
Copy link
Author

Yrisch commented Apr 18, 2023

The fixed values correspond to the default values given in the Interface.py. So in short, in the phantomcode.gas_particles.add_particles method there are 3 calls of set_unit but without the converter values in arguments. The default ones are chosen instead. Resulting in a mismatch between the Amuse converter values and the Phantom values.

@rieder
Copy link
Member

rieder commented Apr 18, 2023

Yes you're right.
Phantom (for now) uses fixed units and should not be called with a converter.
This is a bug - it either should accept and use a converter or it should not accept one.

The reason for this is that Phantom internally uses fixed units for the cooling module - using it with a converter and different units resulted in wrong behaviour. I haven't gotten around to fixing this.

@rieder
Copy link
Member

rieder commented Apr 18, 2023

Before this bug is resolved, at least the following needs to happen:

  • Write a test that runs Phantom without and with (h2) cooling
  • Test with two different converters passed to Phantom
  • Verify the answers are the same (within N decimals) regardless of the converter

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

No branches or pull requests

2 participants