Fix `find_idx` #567

jinningwang · 2024-09-27T20:54:53Z

Fix both ModelData.find_idx() and GroupBase.find_idx()
Add tests on them

jinningwang · 2024-09-27T20:55:57Z

Is the version number messed? Latest document sad v1.9.1+, but release notes have been v1.9.3+

jinningwang · 2024-09-27T21:01:26Z

Do we need to worry following warnings now?

tests/test_case.py::TestPlot::test_kundur_plot
  /Users/jinningwang/work/andes/andes/utils/lazyimport.py:80: DeprecationWarning: Use shutil.which instead of find_executable
    return eval(self.__imported_name__)(*args, **kwargs)

tests/test_case.py::TestCaseInit::test_pvd1_init
  /Users/jinningwang/work/andes/andes/core/model/model.py:754: ComplexWarning: Casting complex values to real discards the imaginary part
    instance.v = np.array(func(*self.s_args[name]),

tests/test_case.py::TestCaseInit::test_pvd1_init
  /Users/jinningwang/work/andes/andes/core/model/model.py:793: ComplexWarning: Casting complex values to real discards the imaginary part
    instance.v[:] = func(*self.s_args[name])

tests/test_known_good.py: 12 warnings
  /Users/jinningwang/work/miniconda3/envs/ams/lib/python3.10/site-packages/dill/_dill.py:1028: DeprecationWarning: numpy.core is deprecated and has been renamed to numpy._core. The numpy._core namespace contains private NumPy internals and its use is discouraged, as NumPy internals can change without warning in any release. In practice, most real-world usage of numpy.core is to access functionality in the public NumPy API. If that is the case, use the public NumPy API. If not, you are using NumPy internals. If you would still like to access an internal attribute, use numpy._core._multiarray_umath.
    submodule = getattr(__import__(module, None, None, [obj]), obj)

jinningwang · 2024-09-27T21:03:16Z

Docker_hub integration needs attentation, https://github.com/CURENT/andes/actions/runs/11077636077/job/30783344184?pr=567

ERROR: failed to solve: python:3.9-slim: failed to resolve source metadata for docker.io/library/python:3.9-slim: failed to authorize: failed to fetch oauth token: unexpected status from GET request to https://auth.docker.io/token?scope=repository%3Alibrary%2Fpython%3Apull&service=registry.docker.io: 401 Unauthorized
Error: Docker build failed with exit code 1

cuihantao · 2024-09-29T15:16:24Z

Jinning,

The warnings can be fixed later, but can you bump up the Python image version in the GitHub actions file to fix the Docker error?

…ibmambapy

jinningwang · 2024-09-29T20:16:44Z

The trouble with github action seems come from the action setup-miniconda@v3 itself:

  /home/runner/miniconda3/condabin/conda config --add channels defaults
  Warning: Error while loading conda entry point: conda-libmamba-solver (module 'libmambapy' has no attribute 'QueryFormat')

Leave it here for now.

jinningwang · 2024-09-30T15:53:32Z

andes/models/group.py

@@ -243,31 +243,41 @@ def set(self, src: str, idx, attr, value):

        return True

-    def find_idx(self, keys, values, allow_none=False, default=None):
+    def find_idx(self, keys, values, allow_none=False, default=None,
+                 no_flatten=False):


no_flatten=False introduces more ambiguity. Remove it since we don't actually need it.
Enforce the return to be a list of lists, thus the behavior is more consistent across models and groups.

jinningwang · 2024-09-30T15:54:51Z

andes/models/group.py

@@ -243,31 +243,41 @@ def set(self, src: str, idx, attr, value):

        return True

-    def find_idx(self, keys, values, allow_none=False, default=None):
+    def find_idx(self, keys, values, allow_none=False, default=None,


default=None can be kept there to be consistent with ModelData.find_idx()

jinningwang · 2024-10-01T13:51:52Z

@cuihantao

I'd like to ask for your suggestions for the below issue.

Problem

sometimes the IdxParam.v can be list of lists, which results in error in System.setup() as the IdxParam.v is expected to be a flatten list.

Minimal code

>>> raw = andes.get_case("kundur/kundur.raw")
>>> dyr = andes.get_case("kundur/kundur_full.dyr")
>>> ss = andes.load(raw, addfile=dyr, setup=False)
>>> ss.GENROU.gen.v
[[1], [2], [3], [4]]

Solution options

Option 1: Add `no_flatten=Falses` in `find_idx()`

Comment: might be confusing as there can be one-to-many mapping

Option 2: Sanitize impacted code (shown below):

andes/andes/system.py

Lines 1768 to 1770 in 5ab784b

    
           for model_idx, dest_idx in zip(model.idx.v, idxp.v): 
        
               if dest_idx not in dest.uid: 
        
                   continue

Comment: IdxParam.v can still be list of lists that breaks the existing users' expectation

Option 3: Add a sanitize step to `IdxParam.v` using `@v.setter`

Comment: Seems to be a good choice to balance the complexity and consistency

cuihantao · 2024-10-01T14:05:08Z

This seems to be a result of the change to “find_idx”. It reminds me that the caller to that function is assuming unique idx. This is the case in GENRIU.gen. To minimize the change in code, maybe we should revert the change to make it behave the old way. You can add a keyword argument, to allow finding all the indices if needed. This will solve the original problem but not impact the existing code. Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Jinning Wang ***@***.***> Sent: Tuesday, October 1, 2024 9:52:13 AM To: CURENT/andes ***@***.***> Cc: Hantao Cui ***@***.***>; Mention ***@***.***> Subject: Re: [CURENT/andes] Fix `find_idx` (PR #567) @cuihantao<https://github.com/cuihantao> I'd like to ask for your suggestions for the below issue. Problem sometimes the IdxParam.v can be list of lists, which results in error in System.setup() as the IdxParam.v is expected to be a flatten list. Minimal code

>> raw = andes.get_case("kundur/kundur.raw") >> dyr = andes.get_case("kundur/kundur_full.dyr") >> ss = andes.load(raw, addfile=dyr, setup=False) >> ss.GENROU.gen.v

[[1], [2], [3], [4]] Solution options Option 1: Add no_flatten=Falses in find_idx() Comment: might be confusing as there can be one-to-many mapping Option 2: Sanitize impacted code (shown below): https://github.com/CURENT/andes/blob/5ab784b75347a98d07c004e32d7c4b39d407ea2e/andes/system.py#L1768-L1770 Comment: IdxParam.v can still be list of lists that breaks the existing users' expectation Option 3: Add a sanitize step to IdxParam.v using @v.setter Comment: Seems to be a good choice to balance the complexity and consistency — Reply to this email directly, view it on GitHub<#567 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABSNZA7YRS56KM52MRVCBCTZZKSI3AVCNFSM6AAAAABO7377CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBWGAYDSNBYHA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

…dx()

jinningwang · 2024-10-03T20:39:41Z

The trouble with github action seems come from the action setup-miniconda@v3 itself:
  /home/runner/miniconda3/condabin/conda config --add channels defaults
  Warning: Error while loading conda entry point: conda-libmamba-solver (module 'libmambapy' has no attribute 'QueryFormat')
Leave it here for now.

This issue seems to have been fixed, conda-incubator/setup-miniconda#366

Additionally, local tests show it works when using mamba-version: "<2.0.0"

jinningwang · 2024-10-04T02:07:58Z

@cuihantao Finally this issue is fixed, I think it's ready to merge

cuihantao · 2024-10-04T02:12:10Z

Thanks Jinning! Just merged it.

jinningwang added 5 commits September 27, 2024 16:30

Fix find_idx to allow multiple matches

ee92612

Add more test on find_idx

9d3ea43

Update release-notes

6da66ea

Minor fix

1e71bf2

Add test on GroupBase.finx_idx

4d69304

Fix deprecateion of np.in1d

1ae1f4e

jinningwang added 7 commits September 29, 2024 15:47

[WIP] Try to fix docker error in github actions

0443b38

[WIP] Undo changes to pythonapp.yml

eb7d23f

[WIP] Fix github action error, add a step to install mamba

e099c9b

[WIP] Fix github action error

71cc0e1

[WIP] Try to fix github action error, reset .yml

45f86d0

[WIP] Try to fix github action error, use classic solver instead of l…

307bfae

…ibmambapy

[WIP] Try to fix github action error

f6ef1e2

jinningwang commented Sep 30, 2024

View reviewed changes

jinningwang added 3 commits October 1, 2024 09:10

Remove no_flatten in find_idx

e5e19d4

Fix tests for find_idx

6abf2b4

Typo

1c489df

jinningwang added 5 commits October 1, 2024 10:13

[WIP] Fix find_idx, revert changes

bccc080

[WIP] Fix find_idx, add parameter allow_all=False to ModelData.find_i…

919c8a4

…dx()

[WIP] Fix find_idx, add inner lists length check

b01fe4d

[WIP] Fix find_idx, refactor input check

e1422b2

[WIP] Fix find_idx, move input check from modeldata to utils.func

fdad124

jinningwang added 4 commits October 3, 2024 11:17

[WIP] Fix find_idx

47e9352

[WIP] Fix find_idx, minor fix

ef34488

[WIP] Fix find_idx, fix invovled tests

33c7ee9

Restore pythonapp workflow

22eceeb

jinningwang added 2 commits October 3, 2024 16:41

Update release notes

9a675f5

Format

3adc110

cuihantao merged commit d534ced into CURENT:develop Oct 4, 2024
3 checks passed

jinningwang deleted the findidx branch October 5, 2024 18:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `find_idx` #567

Fix `find_idx` #567

jinningwang commented Sep 27, 2024

jinningwang commented Sep 27, 2024 •

edited

Loading

jinningwang commented Sep 27, 2024

jinningwang commented Sep 27, 2024

cuihantao commented Sep 29, 2024

jinningwang commented Sep 29, 2024

jinningwang Sep 30, 2024 •

edited

Loading

jinningwang Sep 30, 2024

jinningwang commented Oct 1, 2024

cuihantao commented Oct 1, 2024 via email

jinningwang commented Oct 3, 2024

jinningwang commented Oct 4, 2024

cuihantao commented Oct 4, 2024

Fix find_idx #567

Fix find_idx #567

Conversation

jinningwang commented Sep 27, 2024

jinningwang commented Sep 27, 2024 • edited Loading

jinningwang commented Sep 27, 2024

jinningwang commented Sep 27, 2024

cuihantao commented Sep 29, 2024

jinningwang commented Sep 29, 2024

jinningwang Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

jinningwang Sep 30, 2024

Choose a reason for hiding this comment

jinningwang commented Oct 1, 2024

Problem

Minimal code

Solution options

Option 1: Add no_flatten=Falses in find_idx()

Option 2: Sanitize impacted code (shown below):

Option 3: Add a sanitize step to IdxParam.v using @v.setter

cuihantao commented Oct 1, 2024 via email

jinningwang commented Oct 3, 2024

jinningwang commented Oct 4, 2024

cuihantao commented Oct 4, 2024

Fix `find_idx` #567

Fix `find_idx` #567

jinningwang commented Sep 27, 2024 •

edited

Loading

jinningwang Sep 30, 2024 •

edited

Loading

Option 1: Add `no_flatten=Falses` in `find_idx()`

Option 3: Add a sanitize step to `IdxParam.v` using `@v.setter`