hannorein / rebound

💫 An open-source multi-purpose N-body code.
https://rebound.readthedocs.io/
GNU General Public License v3.0
817 stars 216 forks source link

Read simulationarchives version < 3.18 #778

Closed hannorein closed 1 week ago

hannorein commented 1 week ago

This allows current versions of REBOUND to read old Simulationarchives (version < 3.18).

hannorein commented 1 week ago

@dtamayo I did not expect that this small change breaks anything for REBOUNDx (it should only change something when reading in very old simulation archives). Could it be that there is an issue with CI or REBOUNDx?

dtamayo commented 1 week ago

The REBOUNDx CI has some old binaries that it reads in to test for reproducibility, which I'm guessing are the problem. I can update them. Thanks for letting me know

dtamayo commented 1 week ago

Perhaps this is already what this pull request is fixing, but I noticed when running the SavingAndLoadingSimulations.ipynb example in REBOUNDx (after making a new environment and installing the latest REBOUND from pypi, latest REBOUNDx locally from source with pip install -e .) that when I save a REBOUND binary and reload it

sim.save_to_file("reb.bin") sim2 = rebound.Simulation("reb.bin")

I get these warnings

/Users/dtamayo/opt/miniconda3/envs/rebtest/lib/python3.12/site-packages/rebound/simulation.py:148: RuntimeWarning: Binary file was saved with a different version of REBOUND. Binary format might have changed. warnings.warn(message, RuntimeWarning) /Users/dtamayo/opt/miniconda3/envs/rebtest/lib/python3.12/site-packages/rebound/simulation.py:148: RuntimeWarning: You have to reset function pointers after creating a reb_simulation struct with a binary file. warnings.warn(message, RuntimeWarning) /Users/dtamayo/opt/miniconda3/envs/rebtest/lib/python3.12/site-packages/rebound/simulation.py:148: RuntimeWarning: Encountered unkown field in file. File might have been saved with a different version of REBOUND. warnings.warn(message, RuntimeWarning) /Users/dtamayo/opt/miniconda3/envs/rebtest/lib/python3.12/site-packages/rebound/simulation.py:148: RuntimeWarning: The binary file seems to be corrupted. An attempt has been made to read the uncorrupted parts of it. warnings.warn(message, RuntimeWarning)

I also checked that it isn't anything REBOUNDx is doing with the function pointers (if I don't add reboundx or any forces I still get the same warnings)

hannorein commented 1 week ago

I'm confused.

  1. You're saying that even without this PR and without REBOUNDx something is broken? Can you send me a short piece of code to reproduce that? Just doing

    import rebound
    sim = rebound.Simulation()
    sim.save_to_file("reb.bin")
    sim2 = rebound.Simulation("reb.bin")

    as you wrote works for me.

  2. Where are the old binary files that the REBOUNDx CI is using?

hannorein commented 1 week ago
  1. I think I found them. Are they all in reboundx/test/binaries/? I quickly tested a few and they seem to work. Although some are really old (version 3.9). I'll need to look more into which ones fail...
hannorein commented 1 week ago

Ok. So I think the tests passed by accident before this PR:

dtamayo commented 1 week ago

<Thanks! It looks like the issue causing the warnings are the particle hashes. If I take them out of sim.add, I get no warnings. With hashes, I get the above warnings>

Never mind, need to do some tests to narrow down what's happening. Need to meet wiht a student and will check right after

hannorein commented 1 week ago

Yeah, I can't reproduce that either.

No rush. I also won't have much time this afternoon...

dtamayo commented 1 week ago

If I run the same commands you sent in a jupyter notebook I get the warnings, but not on the command line...

hannorein commented 1 week ago

sigh

hannorein commented 1 week ago

(works for me in a jupyter notebook also)

dtamayo commented 1 week ago

image

This is with python 3.12.4, and just making a new environment and pip install rebound

hannorein commented 1 week ago

Done the same with 3.12.3 and it works. 🤷‍♂️ You're using conda, I'm using venv.. can't think of anything else really...

Screenshot 2024-06-24 at 1 21 21 PM

dtamayo commented 1 week ago

I tried getting python 3.12.3 in conda just in case, but got the same warnings:

image

I also tried using virtualenv instead, both installing the latest pypi version, and checking out the sa16 branch and installing that from source locally (both gave identical outputs)

image

I didn't have exactly the same version of python with virtualenv, but can't see how that would matter...I guess we have different versions of Clang (??)

dtamayo commented 1 week ago

Sigh I forgot that every time you save it appends to the binary. So I think the issue was that I'd run this with an old version of rebound, creating reb.bin, and every time I ran it with a new version, it was appending and on load (correctly) said it was saved with a previous version of rebound.

If I remove the binary and then run it, the warnings disappear

hannorein commented 1 week ago

Ah. Of course! I hadn't thought of that...

So I'll merge this in even though the REBOUNDx machine independence test fails. We can either create a new binary file for the test, or investigate what changed and breaks the test. I think the former is the wiser choice...

dtamayo commented 1 week ago

That makes sense. I've updated REBOUNDx to remove wheels for now, and fixed all the simulationarchive unit tests to regenerate each time. Everything works fine now. There's a bunch of different updates bundled in, so I want to spend a few minutes writing some release notes and I'll push a new version to PyPI tomorrow. Thanks a lot!