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

Added SeisCon copy function #17

Merged
merged 6 commits into from
Aug 27, 2023
Merged

Conversation

kerim371
Copy link
Contributor

This function is required by JUDI's judiVector:
https://github.com/slimgroup/JUDI.jl/blob/16aa54abc9e1a18d8bd59636895afbd90718b0b7/src/TimeModeling/Types/judiVector.jl#L156

This is going to solve the issue: slimgroup/JUDI.jl#181 (comment)

Nevertheless it is not possible to use this function as copy(con) because of the following warning:

WARNING: both SegyIO and Base export "copy"; uses of it in module Main must be qualified

It should be used as SegyIO.copy(con).
Most likely JUDI's judiVector should be edited in appropriate way.

src/types/SeisCon.jl Outdated Show resolved Hide resolved
SegyIO doesn't export `copy` anymore but extends `Base.copy`
@kerim371
Copy link
Contributor Author

Seems to be fixed and working

@kerim371
Copy link
Contributor Author

kerim371 commented Jun 4, 2023

it seems even with this copy function added JUDI still doesn't want to work with SeisCon and filter:
MethodError: no method matching filter!(::SeisCon, ::SeisCon, ::Float32; fmin=0.001f0, fmax=5.0f0)

@mloubout mloubout added the bug Something isn't working label Jun 5, 2023
src/types/BlockScan.jl Outdated Show resolved Hide resolved
@mloubout
Copy link
Member

mloubout commented Jun 5, 2023

no method matching filter!(::SeisCon

That's an issue for JUDI. THis isn't implemented it is currently only supporting in core.

This function is required by JUDI's judiVector:
This line is actually a hack to avoid errors but should throw an error as it will not give the correct result even if copy is implemented.

@mloubout mloubout merged commit 4d74906 into slimgroup:master Aug 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants