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

test_somoclu_summarizers.py failing #300

Closed
maxwest-uw opened this issue Feb 13, 2023 · 7 comments
Closed

test_somoclu_summarizers.py failing #300

maxwest-uw opened this issue Feb 13, 2023 · 7 comments
Labels
bug Something isn't working dependencies Pull requests that update a dependency file

Comments

@maxwest-uw
Copy link
Collaborator

Running an a 2021 macbook M1 (so apple silicon, which might be the cause of my woes...)

when trying to build and run the RAIL package locally, I get the following errors on the somoclu test:

FAILED tests/test_somoclu_summarizers.py::test_SomocluSOM - NameError: name 'wrap_train' is not defined
FAILED tests/test_somoclu_summarizers.py::test_SomocluSOM_with_mag_and_colors - NameError: name 'wrap_train' is not defined

there seems to be a related issue tracked in the somoclu repo noting this on windows machines from 4 years ago. It seems like they pushed a 1.7.5.1 update to fix it but the pip install doesn't actually work locally, due to a metadata issue:

Discarding https://files.pythonhosted.org/packages/bf/1d/8f2a89cd9a0f37fe3a8ba114f43e310a832f5f30f7f3d886e6379ab2e722/somoclu-1.7.5.1.tar.gz (from https://pypi.org/simple/somoclu/): Requested somoclu from file:///Users/maxwest/Library/Caches/pip/wheels/47/89/ab/0932352d0a0a8ceebb91b937bd4de854ec6927f4ab6c78b7b4/somoclu-1.7.5-py3-none-any.whl has inconsistent version: expected '1.7.5.1', but metadata has '1.7.5'

They also have a 1.7.6 release out from ~2 years ago but it's not on pypi 🙁

@maxwest-uw maxwest-uw added bug Something isn't working dependencies Pull requests that update a dependency file labels Feb 13, 2023
@sschmidt23
Copy link
Collaborator

Yes, I think that this is yet another instance of package developers updating conda and not PyPI, if you do a conda install -c conda-forge somoclu it should grab an updated version of the package and things should work. We should add this to install instructions, and maybe ping the somoclu developers to see if they're willing to update the package on PyPI.

@maxwest-uw
Copy link
Collaborator Author

for some reason conda can't find the somoclu package, even though it's listed on conda-forge 🤔 weird, could be a problem with my local setup.

I added a comment to the related issue and I'll probably ask them to put 1.7.6 on pypi.

@eacharles
Copy link
Collaborator

I think the issue here is primarily that some of the underlying packages aren't up to date for M1 mac.

@maxwest-uw
Copy link
Collaborator Author

the somoclu people updated the package on pip and conda, so I have 1.7.6 working on my M1! but the somoclu tests are still failing 🤔

        # test loading model by name rather than via handle
        summarizer2 = summarizer_class.make_stage(
            name=key,
            model=f"tmpsomoclu_"+key+".pkl",
            aliases={
                "model": "somoclu_summarize_test2_model",
                "input": "somoclu_summarize_test2_input",
                "spec_input": "somoclu_summarize_test2_spec_input",
            },
        )
        _ = summarizer2.summarize(phot_data, spec_data)
        fid_ens = qp.read(
            summarizer2.get_output(
                summarizer2.get_aliased_tag("single_NZ"), final_name=True
            )
        )
        meanz = fid_ens.mean().flatten()
>       assert np.isclose(meanz[0], 0.14414913252122552)
E       assert False
E        +  where False = <function isclose at 0x104591550>(0.13940019753973246, 0.14414913252122552)
E        +    where <function isclose at 0x104591550> = np.isclose

tests/test_somoclu_summarizers.py:62: AssertionError

@eacharles
Copy link
Collaborator

eacharles commented Mar 7, 2023 via email

@sschmidt23
Copy link
Collaborator

I think this same issue popped up with pzflow, and if you look at the pzflow test in tests/test_algos.py you can see that we have a np.isclose with an atol=0.05 because there is a minor numerical instability that led to some small changes in the results when some of the normalizing flow dependencies were being updated to newer versions. My guess is that we should just do the same thing here.

@eacharles
Copy link
Collaborator

overtaken by events a while back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

3 participants