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

Compatibility with Ansys 2022 R2 #142

Merged
merged 151 commits into from
Nov 9, 2023
Merged

Compatibility with Ansys 2022 R2 #142

merged 151 commits into from
Nov 9, 2023

Conversation

hzw770
Copy link

@hzw770 hzw770 commented Sep 15, 2022

When running pyepr on ansys 2022 R2, ansys complained that it does not recognize "re(Mode(1)):Curve1". This could be related to the new version's names of the figure properties.

The way I temporarily fix this is to remove ":Curve1", and also comment out several "set_property" commands in core_distributed_analysis.py .

@zlatko-minev
Copy link
Owner

Thank you will need to look at this more carefully

Is it backward compatible?

@hzw770
Copy link
Author

hzw770 commented Sep 15, 2022

I don't currently have old ansys versions installed unfortunately.
But it shouldn't matter if one just comment out line 1579 to 1587 in core_distributed_analysis.py because they just edit the figure properties?

@zlatko-minev
Copy link
Owner

For any of the backward breaking changes you can add an if statement as shown in the ansys.py file that checks the version of ansys and one one thing if old and another if new

Do you know what I mean?

@hzw770
Copy link
Author

hzw770 commented Sep 15, 2022

I see, let me try

@hzw770
Copy link
Author

hzw770 commented Sep 18, 2022

For backward compatibility:
In core_distributed_analysis.py:

First, import Dispatch at the beginning of this .py
from win32com.client import Dispatch

Then replace previous 1579 - 1587, by:

        _app = Dispatch('AnsoftHfss.HfssScriptInterface')
        desktop = _app.GetAppDesktop()
        _ansys_version_ = desktop.GetVersion()
        if _ansys_version_ <= '2022':
            # Properties of lines
            curves = [f"{report_name}:re(Mode({i})):Curve1" for i in range(
                1, 1+self.n_modes)]
            set_property(report, 'Attributes', curves, 'Line Width', 3)
            set_property(report, 'Scaling',
                         f"{report_name}:AxisY1", 'Auto Units', False)
            set_property(report, 'Scaling', f"{report_name}:AxisY1", 'Units', 'g')
            set_property(report, 'Legend',
                         f"{report_name}:Legend", 'Show Solution Name', False)

@zlatko-minev
Copy link
Owner

Is the code backwards compatible now?

@zlatko-minev zlatko-minev self-requested a review December 16, 2022 21:08
nikosavola and others added 2 commits December 19, 2022 16:27
The epr_numerical_diagonalization function uses np.float which has been
deprecated in Numpy.

Refer https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations

The continued usage of np.float yields the following error in the
epr_numerical_diaganolization function when later versions of Numpy is used.

"AttributeError: module 'numpy' has no attribute 'float'."

This commit replace np.float with float to solve this error.
@zlatko-minev
Copy link
Owner

Wondering if it would be safe to merge this into main, what do you think?

clarayfontaine and others added 3 commits April 25, 2023 16:20
* Fix mode selection in analyze_variation

Addressing issue #148: analyze_variation incorrectly chooses Pj, Sj, Om, PHI_zpf when passed a subset of modes

* Remove redundant frequency selection

The frequencies are already correctly selected outside of this if-statement, so this is not necessary
@landamax
Copy link

Hi, only wanted to know whether this issue will be merged to main branch soon?
I really want to use an original version and not to work with some quick-fixes as comment outs and such. Also it looks like this issue breaks down the EPR analysis in Qiskit Metal.

@zlatko-minev
Copy link
Owner

You mean this PR break with Qiskit Metal use. Yes, that means it is not compatible in current form. We would want some help to fix and make not quick issues. I also have some questions in the code here too.

@zlatko-minev
Copy link
Owner

I was also a bit confused before. I now notice that this PR is pulling from main into develop (not into main)

@landamax
Copy link

Oh I didn't even notice that it pulls into develop!
I noticed this issue became more inclusive: it has some fixes for calculation of q factors, specifically the surface q factor (instead of raising a NotImplementedError it actually performs the calculation, which was implemented about a year ago).
Can this issue be pulled into main?

@zlatko-minev
Copy link
Owner

I tried to change it on my end but got:
image

Can you try on your end? Or you can close this one and reopen it with master

@landamax
Copy link

Oh I think I don't have permissions to do something like that, but I see this:
to_pyepr_github

so maybe you can merge the develop branch with master after reviewing this issue.

out-commenting a property not supported in latest version. Only visual impact
@zlatko-minev zlatko-minev merged commit 6dd5dc6 into develop Nov 9, 2023
4 checks passed
@zlatko-minev
Copy link
Owner

Thank you for the effort on this massive update! I just merged! Hopefully there aren't any major compatibility issues, i didn;t catch any here

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

Successfully merging this pull request may close these issues.

None yet